Fix: proxy buffer may be insufficient after PCRE rewrite

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

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; }
@@ -587,6 +607,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
@@ -620,6 +644,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);
  }
 
@@ -1097,6 +1122,7 @@ for(;;){
 	RETURN(0);
  }
  if(action != PASS) RETURN(517);
+ if(growbuf(&buf, &bufsize, inbuf + HDRRESERVE)) RETURN(21);
 
  param->nolongdatfilter = 0;
 
@@ -1120,6 +1146,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;
   }