src/ftppr.c

@@ -64,13 +64,6 @@ void * ftpprchild(struct clientparam* param) {
 
 	}
 	else if (!strncasecmp((char *)buf, "PASS ", 5)){
-		/* The user name carries the server to log in to, and it arrives
-		   with USER. Without it there is nothing to log in to, and what
-		   follows would read the name and the host as if there were. */
-		if(!param->hostname || !param->extusername){
-			socksend(param, param->ctrlsock, (unsigned char *)"503 Login with USER first\r\n", 27, conf.timeouts[STRING_S]);
-			RETURN(805);
-		}
 		param->extpassword = (unsigned char *)strdup((char *)buf+5);
 		inbuf = BUFSIZE;
 		res = ftplogin(param, (char *)buf, &inbuf);

src/httpsrv.c

@@ -937,11 +937,7 @@ 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);
-	/* 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);
+	if(targetunsafe(path) || path[0] != '/') return op_forbidden(r);
 	strcpy(r->path, path);
 	return HTTPSRV_REWRITTEN;
 }
@@ -1331,12 +1327,9 @@ static int httpsrv_request(struct clientparam *param, struct httpreq *r)
 		strcpy(r->path, decoded);
 	}
 
-	/* 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 &&
+	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){
@@ -1391,11 +1384,6 @@ 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.

src/pcre.c

@@ -265,9 +265,6 @@ 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;
@@ -327,17 +324,12 @@ 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++;
@@ -346,13 +338,7 @@ 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){
-			/* 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);
+			newbuf = pl->mallocfunc(ovector[0] + replen + 1);
 			if(!newbuf){
 				pl->freefunc(tmpbuf);
 				return CONTINUE;
@@ -360,7 +346,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 = newsize;
+			*bufsize_p = ovector[0] + replen + 1;
 		}
 		memcpy(*buf_p + ovector[0], tmpbuf, replen);
 		pl->freefunc(tmpbuf);

src/proxy.c

@@ -132,12 +132,6 @@ 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
 
@@ -157,20 +151,6 @@ 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; }
@@ -668,10 +648,6 @@ 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
@@ -705,7 +681,6 @@ 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);
  }
 
@@ -1183,7 +1158,6 @@ for(;;){
 	RETURN(0);
  }
  if(action != PASS) RETURN(517);
- if(growbuf(&buf, &bufsize, inbuf + HDRRESERVE)) RETURN(21);
 
  param->nolongdatfilter = 0;
 
@@ -1207,7 +1181,6 @@ 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;
   }

src/smtpp.c

@@ -159,10 +159,7 @@ void * smtppchild(struct clientparam* param) {
 	i = de64(buf,username,255);
 	if(i < 1) {RETURN(664);}
 	username[i] = 0;
-	/* The name has to carry the host to connect to, and the answer says
-	   whether it did: without one there is nowhere to go, and what follows
-	   reads the name as if there were. */
-	if(parseconnusername((char *)username, param, 0, 587)) {RETURN(669);}
+	parseconnusername((char *)username, param, 0, 587);
 	socksend(param, param->clisock, (unsigned char *)"334 UGFzc3dvcmQ6\r\n", 18,conf.timeouts[STRING_S]);	
 	i = sockgetlinebuf(param, CLIENT, buf, sizeof(buf) - 10, '\n', conf.timeouts[STRING_S]);
 	if(i < 2) {RETURN(665);}
@@ -187,7 +184,7 @@ void * smtppchild(struct clientparam* param) {
 	}
 	if(i < 3 || *username) {RETURN(668);}
 	username[i] = 0;
-	if(parseconnusername((char *)username+1, param, 0, 587)) {RETURN(670);}
+	parseconnusername((char *)username+1, param, 0, 587);
 	res = (int)strlen((char *)username+1) + 2;
 	if(res < i){
 		if(param->extpassword) free(param->extpassword);

tests/cases/pcre.py

@@ -193,43 +193,3 @@ 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.
-    # 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.
-    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")

tests/harness.py

@@ -22,7 +22,6 @@ configurations it needs, starts them, and states what it expects:
 import base64
 import http.client
 import os
-import random
 import shutil
 import socket
 import ssl
@@ -34,11 +33,6 @@ import threading
 import time
 
 
-# Ports handed out in this run: a service which has finished may still be
-# in TIME_WAIT, and another case binding the same port would fail for it.
-_PORTS_TAKEN = set()
-
-
 class Response:
     """A reply, or the reason there wasn't one."""
 
@@ -152,30 +146,14 @@ class Tester:
             sock.close()
 
     def free_port(self):
-        """A port nothing is listening on, and nothing is likely to take.
-
-        Asking the system for an ephemeral port hands back one out of the
-        range it also draws outgoing connections from - 32768 up on Linux,
-        49152 up on Windows - so between the check here and the bind in the
-        service, a connection somewhere else in the suite can take it. That
-        shows up as a service which never listens, or a bind() error deep in
-        a case which has nothing to do with ports. Ports are taken from below
-        both ranges instead, and none is handed out twice in a run.
-        """
-        for _ in range(200):
-            port = random.randint(10000, 19999)
-            if port in _PORTS_TAKEN:
-                continue
-            sock = socket.socket()
-            try:
-                sock.bind(("127.0.0.1", port))
-            except OSError:
-                continue
-            finally:
-                sock.close()
-            _PORTS_TAKEN.add(port)
-            return port
-        raise RuntimeError("no free port in the range the suite uses")
+        """A port nothing is listening on. Closed again before it is used,
+        which is racy in principle and reliable enough in practice."""
+        s = socket.socket()
+        try:
+            s.bind(("127.0.0.1", 0))
+            return s.getsockname()[1]
+        finally:
+            s.close()
 
     def write_config(self, name, config):
         path = os.path.join(self.tmpdir, name + ".cfg")
@@ -380,31 +358,12 @@ class Tester:
             return f"<no reply: {exc}>", 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",
-                   drain=False):
+    def raw_server(self, port, reply, close_after=True, host="127.0.0.1"):
         """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. drain reads the whole request first, however large,
-        which is what a test of the sending side needs.
+        and does not.
         """
         sock = socket.socket()
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@ -419,13 +378,8 @@ class Tester:
                 except OSError:
                     break
                 try:
-                    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.settimeout(self.timeout)
+                    conn.recv(65536)
                     conn.sendall(reply)
                     if close_after:
                         conn.close()