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.
(cherry picked from commit e0f8dc576f62ace9ad1055ca068ab5d4f3a952aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/htx.c b/src/htx.c
index cd21050..ed28deb 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -1068,21 +1068,31 @@
*/
int htx_reqline_to_h1(const struct htx_sl *sl, struct buffer *chk)
{
- if (HTX_SL_LEN(sl) + 4 > b_room(chk))
- return 0;
+ size_t sz = chk->data;
- chunk_memcat(chk, HTX_SL_REQ_MPTR(sl), HTX_SL_REQ_MLEN(sl));
- chunk_memcat(chk, " ", 1);
- chunk_memcat(chk, HTX_SL_REQ_UPTR(sl), HTX_SL_REQ_ULEN(sl));
- 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, HTX_SL_REQ_UPTR(sl), HTX_SL_REQ_ULEN(sl)) ||
+ !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
@@ -1090,20 +1100,31 @@
*/
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
@@ -1112,15 +1133,22 @@
*/
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
@@ -1129,6 +1157,8 @@
*/
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];
@@ -1143,11 +1173,10 @@
*--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))
@@ -1155,4 +1184,8 @@
}
return 1;
+
+ full:
+ chk->data = sz;
+ return 0;
}