BUG/MEDIUM: h1: Don't support LF only to mark the end of a chunk size

It is similar to the previous fix but for the chunk size parsing. But this
one is more annoying because a poorly coded application in front of haproxy
may ignore the last digit before the LF thinking it should be a CR. In this
case it may be out of sync with HAProxy and that could be exploited to
perform some sort or request smuggling attack.

While it seems unlikely, it is safer to forbid LF with CR at the end of a
chunk size.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index 9283df63e4d525964de543667c9acfab03c1b814..7152c6ed82e7d4d2415bd2ab24003ef8973f79dd 100644 (file)
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -303,14 +303,12 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
         * for the end of chunk size.
         */
        while (1) {
-               if (likely(HTTP_IS_CRLF(*ptr))) {
-                       /* we now have a CR or an LF at ptr */
-                       if (likely(*ptr == '\r')) {
-                               if (++ptr >= end)
-                                       ptr = b_orig(buf);
-                               if (--stop == 0)
-                                       return 0;
-                       }
+               if (likely(*ptr == '\r')) {
+                       /* we now have a CR, it must be followed by a LF */
+                       if (++ptr >= end)
+                               ptr = b_orig(buf);
+                       if (--stop == 0)
+                               return 0;
 
                        if (*ptr != '\n')
                                goto error;

diff --git a/src/h1_htx.c b/src/h1_htx.c
index b8c6ccb0567972ce7b5830627631ffc045cd376d..aed1714affa7845116fe07f159ea354538329c0c 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -693,11 +693,6 @@ static size_t h1_parse_full_contig_chunks(struct h1m *h1m, struct htx **dsthtx,
                                ++ridx;
                                break;
                        }
-                       else if (end[ridx] == '\n') {
-                               /* Parse LF only, nothing more to do */
-                               ++ridx;
-                               break;
-                       }
                        else if (likely(end[ridx] == ';')) {
                                /* chunk extension, ends at next CRLF */
                                if (!++ridx)
@@ -710,7 +705,7 @@ static size_t h1_parse_full_contig_chunks(struct h1m *h1m, struct htx **dsthtx,
                                continue;
                        }
                        else {
-                               /* all other characters are unexpected */
+                               /* all other characters are unexpected, especially LF alone */
                                goto parsing_error;
                        }
                }