Fix rewrite and over-long header handling on Windows

op_rewrite checked what it built with targetunsafe(), which describes a
path on this machine: on Windows it must name a drive or a share, so a
rewritten request path was refused and every rewrite rule failed there.
A rewrite produces a request path and is checked as one.

The header loop stopped at HTTPSRV_MAXHDR and answered anyway, leaving
the rest of the request in the stream for the next one to be read out
of. It refuses the request instead. A header longer than the buffer
arrives as several lines, so the count bounds what is read rather than
what a client may send in one header.

The test for the buffer growth after a PCRE rewrite sent its request
through to an httpsrv origin, which stops reading at that same cap; it
uses an origin which reads whatever it is sent, since what is under test
is the proxy in the middle.

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.

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.
+    # 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, url + "/echo", extra=big, body="z")
+        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.http(url + "/echo", proxy=p), "path=/echo",
+        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.

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()