This is an automated email from the ASF dual-hosted git repository. bcall pushed a commit to branch 8.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit c786ebabd77cffd21c3a0e61f8d1afbfa64f1673 Author: Chris Lemmons <alfic...@gmail.com> AuthorDate: Tue Jul 31 17:51:53 2018 +0000 Fixed url_sig error when storing base64ed params in penultimate segment. (cherry picked from commit 2f1dec1e3759c87bf4bdcbcfbae762993d60bbcf) --- plugins/experimental/url_sig/url_sig.c | 111 ++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 28 deletions(-) diff --git a/plugins/experimental/url_sig/url_sig.c b/plugins/experimental/url_sig/url_sig.c index 65c36ac..6aa1fe0 100644 --- a/plugins/experimental/url_sig/url_sig.c +++ b/plugins/experimental/url_sig/url_sig.c @@ -330,14 +330,38 @@ getAppQueryString(const char *query_string, int query_length) } } +/** fixedBufferWrite safely writes no more than *dest_len bytes to *dest_end + * from src. If copying src_len bytes to *dest_len would overflow, it returns + * zero. *dest_end is advanced and *dest_len is decremented to account for the + * written data. No null-terminators are written automatically (though they + * could be copied with data). + */ +static int +fixedBufferWrite(char **dest_end, int *dest_len, const char *src, int src_len) +{ + if (src_len > *dest_len) { + return 0; + } + memcpy(*dest_end, src, src_len); + *dest_end += src_len; + *dest_len -= src_len; + return 1; +} + static char * urlParse(char *url, char *anchor, char *new_path_seg, int new_path_seg_len, char *signed_seg, unsigned int signed_seg_len) { char *segment[MAX_SEGMENTS]; unsigned char decoded_string[2048] = {'\0'}; - char new_url[8192] = {'\0'}; + char new_url[8192]; /* new_url is not null_terminated */ char *p = NULL, *sig_anchor = NULL, *saveptr = NULL; - int i = 0, numtoks = 0, cp_len = 0, l, decoded_len = 0, sig_anchor_seg = 0; + int i = 0, numtoks = 0, decoded_len = 0, sig_anchor_seg = 0; + + char *new_url_end = new_url; + int new_url_len_left = sizeof(new_url); + + char *new_path_seg_end = new_path_seg; + int new_path_seg_len_left = new_path_seg_len; char *skip = strchr(url, ':'); if (!skip || skip[1] != '/' || skip[2] != '/') { @@ -345,8 +369,11 @@ urlParse(char *url, char *anchor, char *new_path_seg, int new_path_seg_len, char } skip += 3; // preserve the scheme in the new_url. - strncat(new_url, url, skip - url); - TSDebug(PLUGIN_NAME, "%s:%d - new_url: %s\n", __FILE__, __LINE__, new_url); + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, url, skip - url)) { + TSError("insufficient space to copy schema into new_path_seg buffer."); + return NULL; + } + TSDebug(PLUGIN_NAME, "%s:%d - new_url: %*s\n", __FILE__, __LINE__, (int)(new_url_end - new_url), new_url); // parse the url. if ((p = strtok_r(skip, "/", &saveptr)) != NULL) { @@ -387,19 +414,18 @@ urlParse(char *url, char *anchor, char *new_path_seg, int new_path_seg_len, char // last path segment so skip them. continue; } - l = strlen(segment[i]); - if (l + 1 > new_path_seg_len) { - TSError("insuficient space to copy into new_path_seg buffer."); + if (!fixedBufferWrite(&new_path_seg_end, &new_path_seg_len_left, segment[i], strlen(segment[i]))) { + TSError("insufficient space to copy into new_path_seg buffer."); return NULL; - } else { - memcpy(new_path_seg, segment[i], l); - new_path_seg[l] = '\0'; - if (i != numtoks - 1) { - strncat(new_path_seg, "/", 1); + } + if (i != numtoks - 1) { + if (!fixedBufferWrite(&new_path_seg_end, &new_path_seg_len_left, "/", 1)) { + TSError("insufficient space to copy into new_path_seg buffer."); + return NULL; } - cp_len += l + 1; } } + *new_path_seg_end = '\0'; TSDebug(PLUGIN_NAME, "new_path_seg: %s", new_path_seg); // save the encoded signing parameter data @@ -434,24 +460,53 @@ urlParse(char *url, char *anchor, char *new_path_seg, int new_path_seg_len, char } TSDebug(PLUGIN_NAME, "decoded_string: %s", decoded_string); - for (i = 0; i < numtoks; i++) { - // cp the base64 decoded string. - if (i == sig_anchor_seg && sig_anchor != NULL) { - memcpy(new_url, segment[i], strlen(segment[i])); - memcpy(new_url, (char *)decoded_string, strlen((char *)decoded_string)); - strncat(new_url, "/", 1); - continue; - } else if (i == numtoks - 2 && sig_anchor == NULL) { - memcpy(new_url, (char *)decoded_string, strlen((char *)decoded_string)); - strncat(new_url, "/", 1); - continue; + { + int oob = 0; /* Out Of Buffer */ + + for (i = 0; i < numtoks; i++) { + // cp the base64 decoded string. + if (i == sig_anchor_seg && sig_anchor != NULL) { + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, segment[i], strlen(segment[i]))) { + oob = 1; + break; + } + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, (char *)decoded_string, strlen((char *)decoded_string))) { + oob = 1; + break; + } + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, "/", 1)) { + oob = 1; + break; + } + + continue; + } else if (i == numtoks - 2 && sig_anchor == NULL) { + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, (char *)decoded_string, strlen((char *)decoded_string))) { + oob = 1; + break; + } + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, "/", 1)) { + oob = 1; + break; + } + continue; + } + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, segment[i], strlen(segment[i]))) { + oob = 1; + break; + } + if (i < numtoks - 1) { + if (!fixedBufferWrite(&new_url_end, &new_url_len_left, "/", 1)) { + oob = 1; + break; + } + } } - memcpy(new_url, segment[i], strlen(segment[i])); - if (i < numtoks - 1) { - strncat(new_url, "/", 1); + if (oob) { + TSError("insufficient space to copy into new_url."); } } - return TSstrndup(new_url, strlen(new_url)); + return TSstrndup(new_url, new_url_end - new_url); } TSRemapStatus