BUG/MEDIUM: http: Don't parse chunked body if there is no input data

With recent modifications on the buffers API, when a buffer is released (calling
b_free), we replace it by BUF_NULL where the area pointer is NULL. So many
operations, like b_peek, must be avoided on a released or not allocated
buffer. These changes were mainly made in the commit c9fa048 ("MAJOR: buffer:
finalize buffer detachment").

Since this commit, HAProxy can crash during the body parsing of chunked HTTP
messages because there is no check on the channel's buffer in HTTP analyzers
(http_request_forward_body and http_response_forward_body) nor in H1 functions
reponsible to parse chunked content (h1_skip_chunk_crlf & co). If a stream is
woken up after all input data were forwarded, its input channel's buffer is
released (so set to BUF_NULL). In this case, if we resume the parsing of a
chunk, HAProxy crashes.

To fix this issue, we just skip the parsing of chunks if there is no input data
for the corresponding channel. This is only done if the message state is
strickly lower to HTTP_MSG_ENDING.

diff --git a/src/proto_http.c b/src/proto_http.c
index c17d67ca39705fa84ed6c3a853270bb7c4ed57e9..9f2ccc01dee951260ca140b9eeb30fbed55f4afc 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -5763,6 +5763,13 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
         * HTTP_MSG_DATA, HTTP_MSG_CHUNK_SIZE, HTTP_MSG_CHUNK_CRLF,
         * HTTP_MSG_TRAILERS or HTTP_MSG_ENDING. */
 
+       if (msg->msg_state == HTTP_MSG_ENDING)
+               goto ending;
+
+       /* Don't parse chunks if there is no input data */
+       if (!ci_data(chn))
+               goto waiting;
+
   switch_states:
        switch (msg->msg_state) {
                case HTTP_MSG_DATA:
@@ -5836,9 +5843,6 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
                                goto missing_data_or_waiting;
                        break;
 
-               case HTTP_MSG_ENDING:
-                       goto ending;
-
                default:
                        /* This should no happen in this function */
                        goto error;