BUG/CRITICAL: cache: Fix trivial crash by sending accept-encoding header
Since commit 3d08236cb344607557f22e00cf1cc3e321a1fa95 HAProxy can be trivially
crashed remotely by sending an `accept-encoding` HTTP request header that
contains 16 commas.
This is because the `values` array in `accept_encoding_normalizer` accepts only
16 entries and it is not verified whether the end is reached during looping.
Fix this issue by checking the length. This patch also simplifies the ist
processing in the loop, because it manually calculated offsets and lengths,
when the ist API exposes perfectly safe functions to advance and truncate ists.
I wonder whether the accept_encoding_normalizer function is able to re-use some
existing function for parsing headers that may contain lists of values. I'll
leave this evaluation up to someone else, only patching the obvious crash.
This commit is 2.4-dev specific and was merged just a few hours ago. No
backport needed.
diff --git a/src/cache.c b/src/cache.c
index bdad8fd..6489f16 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1797,6 +1797,7 @@
return istdiff(ist_a, ist_b);
}
+#define ACCEPT_ENCODING_MAX_ENTRIES 16
/*
* Build a hash of the accept-encoding header. The different parts of the
* header value are first sorted, appended and then a crc is calculated
@@ -1805,20 +1806,24 @@
*/
static int accept_encoding_normalizer(struct ist value, char *buf, unsigned int *buf_len)
{
- int retval = 0;
- struct ist values[16] = {{}};
- unsigned int count = 0;
+ struct ist values[ACCEPT_ENCODING_MAX_ENTRIES] = {{}};
+ size_t count = 0;
char *comma = NULL;
struct buffer *trash = get_trash_chunk();
int hash_value = 0;
/* The hash will be built out of a sorted list of accepted encodings. */
- while((comma = istchr(value, ',')) != NULL) {
- values[count++] = ist2(istptr(value), comma-istptr(value));
- value = ist2(comma+1, istlen(value) - (comma-istptr(value)) - 1);
+ while (count < (ACCEPT_ENCODING_MAX_ENTRIES - 1) && (comma = istchr(value, ',')) != NULL) {
+ size_t length = comma - istptr(value);
+
+ values[count++] = isttrim(value, length);
+ value = istadv(value, length + 1);
}
values[count++] = value;
+ if (count == ACCEPT_ENCODING_MAX_ENTRIES)
+ return 1;
+
/* Sort the values alphabetically. */
qsort(values, count, sizeof(struct ist), &accept_encoding_cmp);
@@ -1830,8 +1835,9 @@
memcpy(buf, &hash_value, sizeof(hash_value));
*buf_len = sizeof(hash_value);
- return retval;
+ return 0;
}
+#undef ACCEPT_ENCODING_MAX_ENTRIES
/*
* Normalizer used by default for User-Agent and Referer headers. It only