From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 14 Oct 2019 12:36:51 +0000 (+0200)
Subject: BUG/MEDIUM: htx: Catch chunk_memcat() failures when HTX data are formatted to h1
X-Git-Tag: v2.1-dev3~80
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e0f8dc576f62ace9ad1055ca068ab5d4f3a952aa;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: htx: Catch chunk_memcat() failures when HTX data are formatted to h1

In functions htx_*_to_h1(), most of time several calls to chunk_memcat() are
chained. The expected size is always compared to available room in the buffer to
be sure the full copy will succeed. But it is a bit risky because it relies on
the fact the function chunk_memcat() evaluates the available room in the buffer
in a same way than htx ones. And, unfortunately, it does not. A bug in
chunk_memcat() will always leave a byte unused in the buffer. So, for instance,
when a chunk is copied in an almost full buffer, the last CRLF may be skipped.

To fix the issue, we now rely on the result of chunk_memcat() only.

This patch must be backported to 2.0 and 1.9.
---

diff --git a/src/htx.c b/src/htx.c
index c9671c12fc..bdca69b380 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -1047,9 +1047,7 @@ void htx_move_blk_before(struct htx *htx, struct htx_blk **blk, struct htx_blk *
 int htx_reqline_to_h1(const struct htx_sl *sl, struct buffer *chk)
 {
 	struct ist uri;
-
-	if (HTX_SL_LEN(sl) + 4 > b_room(chk))
-		return 0;
+	size_t sz = chk->data;
 
 	uri = htx_sl_req_uri(sl);
 	if (sl->flags & HTX_SL_F_NORMALIZED_URI) {
@@ -1062,18 +1060,29 @@ int htx_reqline_to_h1(const struct htx_sl *sl, struct buffer *chk)
 		}
 	}
 
-	chunk_memcat(chk, HTX_SL_REQ_MPTR(sl), HTX_SL_REQ_MLEN(sl));
-	chunk_memcat(chk, " ", 1);
-	chunk_memcat(chk, uri.ptr, uri.len);
-	chunk_memcat(chk, " ", 1);
-	if (sl->flags & HTX_SL_F_VER_11)
-		chunk_memcat(chk, "HTTP/1.1", 8);
-	else
-		chunk_memcat(chk, HTX_SL_REQ_VPTR(sl), HTX_SL_REQ_VLEN(sl));
+	if (!chunk_memcat(chk, HTX_SL_REQ_MPTR(sl), HTX_SL_REQ_MLEN(sl)) ||
+	    !chunk_memcat(chk, " ", 1) ||
+	    !chunk_memcat(chk, uri.ptr, uri.len) ||
+	    !chunk_memcat(chk, " ", 1))
+		goto full;
 
-	chunk_memcat(chk, "\r\n", 2);
+	if (sl->flags & HTX_SL_F_VER_11) {
+		if (!chunk_memcat(chk, "HTTP/1.1", 8))
+			goto full;
+	}
+	else {
+		if (!chunk_memcat(chk, HTX_SL_REQ_VPTR(sl), HTX_SL_REQ_VLEN(sl)))
+			goto full;
+	}
+
+	if (!chunk_memcat(chk, "\r\n", 2))
+		goto full;
 
 	return 1;
+
+  full:
+	chk->data = sz;
+	return 0;
 }
 
 /* Appends the H1 representation of the status line <sl> to the chunk <chk>. It
@@ -1081,20 +1090,31 @@ int htx_reqline_to_h1(const struct htx_sl *sl, struct buffer *chk)
  */
 int htx_stline_to_h1(const struct htx_sl *sl, struct buffer *chk)
 {
-	if (HTX_SL_LEN(sl) + 4 > b_size(chk))
+	size_t sz = chk->data;
+
+	if (HTX_SL_LEN(sl) + 4 > b_room(chk))
 		return 0;
 
-	if (sl->flags & HTX_SL_F_VER_11)
-		chunk_memcat(chk, "HTTP/1.1", 8);
-	else
-		chunk_memcat(chk, HTX_SL_RES_VPTR(sl), HTX_SL_RES_VLEN(sl));
-	chunk_memcat(chk, " ", 1);
-	chunk_memcat(chk, HTX_SL_RES_CPTR(sl), HTX_SL_RES_CLEN(sl));
-	chunk_memcat(chk, " ", 1);
-	chunk_memcat(chk, HTX_SL_RES_RPTR(sl), HTX_SL_RES_RLEN(sl));
-	chunk_memcat(chk, "\r\n", 2);
+	if (sl->flags & HTX_SL_F_VER_11) {
+		if (!chunk_memcat(chk, "HTTP/1.1", 8))
+			goto full;
+	}
+	else {
+		if (!chunk_memcat(chk, HTX_SL_RES_VPTR(sl), HTX_SL_RES_VLEN(sl)))
+			goto full;
+	}
+	if (!chunk_memcat(chk, " ", 1) ||
+	    !chunk_memcat(chk, HTX_SL_RES_CPTR(sl), HTX_SL_RES_CLEN(sl)) ||
+	    !chunk_memcat(chk, " ", 1) ||
+	    !chunk_memcat(chk, HTX_SL_RES_RPTR(sl), HTX_SL_RES_RLEN(sl)) ||
+	    !chunk_memcat(chk, "\r\n", 2))
+		goto full;
 
 	return 1;
+
+  full:
+	chk->data = sz;
+	return 0;
 }
 
 /* Appends the H1 representation of the header <n> witht the value <v> to the
@@ -1103,15 +1123,22 @@ int htx_stline_to_h1(const struct htx_sl *sl, struct buffer *chk)
  */
 int htx_hdr_to_h1(const struct ist n, const struct ist v, struct buffer *chk)
 {
+	size_t sz = chk->data;
+
 	if (n.len + v.len + 4 > b_room(chk))
 		return 0;
 
-	chunk_memcat(chk, n.ptr, n.len);
-	chunk_memcat(chk, ": ", 2);
-	chunk_memcat(chk, v.ptr, v.len);
-	chunk_memcat(chk, "\r\n", 2);
+	if (!chunk_memcat(chk, n.ptr, n.len) ||
+	    !chunk_memcat(chk, ": ", 2) ||
+	    !chunk_memcat(chk, v.ptr, v.len) ||
+	    !chunk_memcat(chk, "\r\n", 2))
+		goto full;
 
 	return 1;
+
+  full:
+	chk->data = sz;
+	return 0;
 }
 
 /* Appends the H1 representation of the data <data> to the chunk <chk>. If
@@ -1120,6 +1147,8 @@ int htx_hdr_to_h1(const struct ist n, const struct ist v, struct buffer *chk)
  */
 int htx_data_to_h1(const struct ist data, struct buffer *chk, int chunked)
 {
+	size_t sz = chk->data;
+
 	if (chunked) {
 		uint32_t chksz;
 		char     tmp[10];
@@ -1134,11 +1163,10 @@ int htx_data_to_h1(const struct ist data, struct buffer *chk, int chunked)
 			*--beg = hextab[chksz & 0xF];
 		} while (chksz >>= 4);
 
-		if (data.len + (end - beg) + 2 > b_room(chk))
-			return 0;
-		chunk_memcat(chk, beg, end - beg);
-		chunk_memcat(chk, data.ptr, data.len);
-		chunk_memcat(chk, "\r\n", 2);
+		if (!chunk_memcat(chk, beg, end - beg) ||
+		    !chunk_memcat(chk, data.ptr, data.len) ||
+		    !chunk_memcat(chk, "\r\n", 2))
+			goto full;
 	}
 	else {
 		if (!chunk_memcat(chk, data.ptr, data.len))
@@ -1146,4 +1174,8 @@ int htx_data_to_h1(const struct ist data, struct buffer *chk, int chunked)
 	}
 
 	return 1;
+
+  full:
+	chk->data = sz;
+	return 0;
 }