diff --git a/src/httpsrv.c b/src/httpsrv.c index de2b6ec..6a25589 100644 --- a/src/httpsrv.c +++ b/src/httpsrv.c @@ -937,7 +937,11 @@ static int op_rewrite(struct httpreq *r, const unsigned char *params) if(!params || !*params) return op_forbidden(r); if(expand(path, sizeof(path), params, r->path, r->caps, r->ncaps)) return op_forbidden(r); - if(targetunsafe(path) || path[0] != '/') return op_forbidden(r); + /* What comes out is a request path and is checked as one. A path on this + machine is a different thing, and asking a rewrite to look like one + would leave nothing writable on Windows, where such a path names a + drive or a share. */ + if(path[0] != '/' || pathunsafe(path)) return op_forbidden(r); strcpy(r->path, path); return HTTPSRV_REWRITTEN; } @@ -1327,9 +1331,12 @@ static int httpsrv_request(struct clientparam *param, struct httpreq *r) strcpy(r->path, decoded); } - while(hdrs++ < HTTPSRV_MAXHDR && + /* A header longer than the buffer arrives as several lines, so the count + bounds what is read and not what a client may send in one header. */ + while(hdrs < HTTPSRV_MAXHDR && (i = sockgetlinebuf(param, CLIENT, (unsigned char *)buf, sizeof(buf) - 1, '\n', conf.timeouts[STRING_S])) > 2){ + hdrs++; if(rawkeep(r, buf, i)) RETURN(710); buf[i] = 0; if(!strncasecmp(buf, "host:", 5) && !r->proxy){ @@ -1384,6 +1391,11 @@ static int httpsrv_request(struct clientparam *param, struct httpreq *r) } } + /* The headers ran past what this server reads, so the rest of the request + is still in the stream and there is no answering it: what follows would + be read as the request after this one. */ + if(hdrs >= HTTPSRV_MAXHDR) RETURN(712); + /* The next request begins where this body ends, so a body which cannot be read to its end - one this server does not frame, or one longer than it is willing to read - closes the connection instead. diff --git a/tests/cases/pcre.py b/tests/cases/pcre.py index 0a10059..5af80da 100644 --- a/tests/cases/pcre.py +++ b/tests/cases/pcre.py @@ -198,14 +198,24 @@ def run(t): # 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") + # The origin here reads whatever it is sent and answers the same way every + # time: what is being tested is the proxy in the middle, not what a server + # is willing to accept in one request. + grown = t.free_port() + stop = t.raw_server(grown, b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok", + drain=True) + try: + 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, f"http://127.0.0.1:{grown}/x", + extra=big, body="z") + t.contains(reply, "200", "a doubled header block with a body is answered") + t.contains(t.raw_proxy_request(p, f"http://127.0.0.1:{grown}/x"), "200", + "and the proxy is still there afterwards") + finally: + stop() # 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. diff --git a/tests/harness.py b/tests/harness.py index 2259780..8912b69 100644 --- a/tests/harness.py +++ b/tests/harness.py @@ -375,12 +375,14 @@ class Tester: 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"): + def raw_server(self, port, reply, close_after=True, host="127.0.0.1", + drain=False): """Answer every connection with fixed bytes. Returns a stop function. For the shapes a real server would have to be talked into: an answer whose body is delimited by the close, or one which promises to stay - and does not. + and does not. drain reads the whole request first, however large, + which is what a test of the sending side needs. """ sock = socket.socket() sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) @@ -395,8 +397,13 @@ class Tester: except OSError: break try: - conn.settimeout(self.timeout) - conn.recv(65536) + conn.settimeout(0.5 if drain else self.timeout) + while True: + try: + if not conn.recv(65536) or not drain: + break + except socket.timeout: + break # it has stopped sending conn.sendall(reply) if close_after: conn.close()