diff --git a/src/pcre.c b/src/pcre.c index b7f84ba..9c2fdc5 100644 --- a/src/pcre.c +++ b/src/pcre.c @@ -265,6 +265,9 @@ static FILTER_ACTION pcre_filter_client(void *fo, struct clientparam * param, vo return (res)? CONTINUE:PASS; } +/* What a rewritten buffer keeps free for its caller to append to. */ +#define PCRE_HEADROOM 1024 + static FILTER_ACTION pcre_filter_buffer(void *fc, struct clientparam *param, unsigned char ** buf_p, int * bufsize_p, int offset, int * length_p){ PCRE2_SIZE *ovector; int count = 0; @@ -324,12 +327,17 @@ static FILTER_ACTION pcre_filter_buffer(void *fc, struct clientparam *param, uns else if(*replace == '$' && isnumber(*(replace+1))){ replace ++; num = atoi(replace); + /* Past the digits first, and only then decide whether + the group is one to copy: the pass which measured + this string did it in that order, and a reference it + counted as nothing must not be written out as its + own digits here. */ + while(isnumber(*replace)) replace++; if(num > (count - 1)) continue; if(ovector[(num<<1)] == PCRE2_UNSET) continue; if(ovector[(num<<1) + 1] > (PCRE2_SIZE)*length_p || ovector[(num<<1)] > ovector[(num<<1) + 1]) continue; memcpy(target, *buf_p + ovector[(num<<1)], ovector[(num<<1) + 1] - ovector[(num<<1)]); target += (ovector[(num<<1) + 1] - ovector[(num<<1)]); - while(isnumber(*replace)) replace++; } else { *target++ = *replace++; @@ -338,7 +346,13 @@ static FILTER_ACTION pcre_filter_buffer(void *fc, struct clientparam *param, uns repsz = (int)(target - tmpbuf); memcpy(target, *buf_p + ovector[1], *length_p - ovector[1]); if((ovector[0] + replen + 1) > *bufsize_p){ - newbuf = pl->mallocfunc(ovector[0] + replen + 1); + /* Room beyond what was produced: whoever asked for the + filtering usually has something of its own to add, and a + buffer sized to the last byte written leaves nowhere to + put it. The size reported is the size allocated. */ + int newsize = ovector[0] + replen + 1 + PCRE_HEADROOM; + + newbuf = pl->mallocfunc(newsize); if(!newbuf){ pl->freefunc(tmpbuf); return CONTINUE; @@ -346,7 +360,7 @@ static FILTER_ACTION pcre_filter_buffer(void *fc, struct clientparam *param, uns memcpy(newbuf, *buf_p, ovector[0]); pl->freefunc(*buf_p); *buf_p = (unsigned char *)newbuf; - *bufsize_p = ovector[0] + replen + 1; + *bufsize_p = newsize; } memcpy(*buf_p + ovector[0], tmpbuf, replen); pl->freefunc(tmpbuf); diff --git a/src/proxy.c b/src/proxy.c index 83a0a32..a52d972 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -132,6 +132,12 @@ char * proxy_stringtable[] = { }; #define LINESIZE 32768 +/* "Content-Length: " plus 20 digits plus CRLF and a NUL, rounded up */ +#define CLHDRSIZE 48 +/* what the headers this proxy adds of its own can come to: a Forwarded or + Via with a host name in it, a Connection, a Proxy-support and a + Proxy-Authorization carrying an encoded user and password */ +#define HDRRESERVE 2048 #define BUFSIZE (LINESIZE*2) #define FTPBUFSIZE 1536 @@ -151,6 +157,20 @@ static int send_st(struct clientparam *param, int idx){ return socksend(param, param->clisock, (unsigned char *)proxy_stringtable[idx], pst_len(idx), conf.timeouts[STRING_S]); } +/* Makes room in a buffer whose size is tracked. A filter may hand back one + holding exactly what it produced, so nothing may be added to it without + asking for the room first. Returns 1 when the room cannot be had. */ +static int growbuf(unsigned char **buf, int *bufsize, int need){ + unsigned char *newbuf; + + if(need <= *bufsize) return 0; + need += BUFSIZE; /* for what follows too, not just this */ + if(!(newbuf = realloc(*buf, need))) return 1; + *buf = newbuf; + *bufsize = need; + return 0; +} + static void freeptr(void *p){ void **pp = (void **)p; if(*pp) { free(*pp); *pp = NULL; } @@ -648,6 +668,10 @@ for(;;){ RETURN(0); } if(action != PASS) RETURN(517); + /* A filter may have returned a buffer sized to exactly what it produced. + The headers this proxy adds of its own go in after it, so the room for + them is taken back before anything is written. */ + if(growbuf(&buf, &bufsize, inbuf + HDRRESERVE)) RETURN(21); param->nolongdatfilter = 0; #endif @@ -681,6 +705,7 @@ for(;;){ contentlength64 = param->cliinbuf; param->nolongdatfilter = 1; } + if(growbuf(&buf, &bufsize, (int)strlen((char *)buf) + CLHDRSIZE)) RETURN(21); sprintf((char*)buf+strlen((char *)buf), "Content-Length: %"PRIu64"\r\n", contentlength64); } @@ -1158,6 +1183,7 @@ for(;;){ RETURN(0); } if(action != PASS) RETURN(517); + if(growbuf(&buf, &bufsize, inbuf + HDRRESERVE)) RETURN(21); param->nolongdatfilter = 0; @@ -1181,6 +1207,7 @@ for(;;){ } if(action != PASS) RETURN(517); contentlength64 = param->srvinbuf; + if(growbuf(&buf, &bufsize, (int)strlen((char *)buf) + CLHDRSIZE)) RETURN(21); sprintf((char*)buf+strlen((char *)buf), "Content-Length: %"PRIu64"\r\n", contentlength64); hascontent = 1; } diff --git a/tests/cases/pcre.py b/tests/cases/pcre.py index 1e37134..0a10059 100644 --- a/tests/cases/pcre.py +++ b/tests/cases/pcre.py @@ -193,3 +193,33 @@ def run(t): r = t.http(url + "/echo/old", proxy=p) t.eq(200, r.status, "a rewritten request through a parent arrives") t.contains(r, "path=/echo/new", "the origin sees the rewritten path through a parent") + + # --- a rewrite which grows the headers ---------------------------------- + # GHSA-h845-prxq-ww3q: a rewrite that doubles the client headers used to + # leave a buffer holding exactly what it produced, and the Content-Length + # the data filter regenerates was then written past the end of it. + p = proxy_with("rewrite_grow", + 'pcre_rewrite cliheader dunno "(?s).*" "$0$0"', + 'pcre clidata dunno *') + big = "".join("X-%d: %s\r\n" % (i, chr(65 + i) * 20000) for i in range(5)) + reply = t.raw_proxy_request(p, url + "/echo", extra=big, body="z") + t.contains(reply, "200", "a doubled header block with a body is answered") + t.contains(t.http(url + "/echo", proxy=p), "path=/echo", + "and the proxy is still there afterwards") + + # A reference to a group the pattern does not have is dropped, and dropped + # by both the pass which measures the result and the pass which writes it. + p = proxy_with("rewrite_nogroup", + 'pcre_rewrite cliheader dunno "(?s)Host:" "$9$9$9$9$9$9$9$9"') + r = t.http(url + "/echo", proxy=p, headers={"X-Pad": "P" * 2000}) + t.eq(200, r.status, "a reference to a group which did not match is left out") + t.contains(t.http(url + "/echo", proxy=p), "path=/echo", + "and that proxy is still there too") + + # an optional group which took part on one request and not on the next + p = proxy_with("rewrite_optgroup", + 'pcre_rewrite cliheader dunno "X-Mark: (a)?(b)" "[$1][$2]"') + t.eq(200, t.http(url + "/echo", proxy=p, headers={"X-Mark": "ab"}).status, + "a group which matched is put in") + t.eq(200, t.http(url + "/echo", proxy=p, headers={"X-Mark": "b"}).status, + "and one which did not is left out") diff --git a/tests/harness.py b/tests/harness.py index b275c60..2259780 100644 --- a/tests/harness.py +++ b/tests/harness.py @@ -358,6 +358,23 @@ class Tester: return f"", True return b"".join(chunks).decode("utf-8", "replace"), closed + def raw_proxy_request(self, proxy, url, extra="", body="", method=None): + """Send one absolute-URI request through a proxy, headers and all. + + For the requests a client library will not send: an oversized header + block, or one whose exact bytes matter. + """ + phost, pport = self._hostport(proxy) + host, port, path = self._split(url) + method = method or ("POST" if body else "GET") + request = (f"{method} http://{host}:{port}{path} HTTP/1.1\r\n" + f"Host: {host}:{port}\r\n" + extra) + if body: + request += f"Content-Length: {len(body)}\r\n" + request += "\r\n" + body + text, _ = self.raw_session(pport, request, host=phost, quiet=2) + return text + def raw_server(self, port, reply, close_after=True, host="127.0.0.1"): """Answer every connection with fixed bytes. Returns a stop function.