diff options
author | Jay Satiro <raysatiro@yahoo.com> | 2016-03-03 01:24:27 -0500 |
---|---|---|
committer | Jay Satiro <raysatiro@yahoo.com> | 2016-04-11 22:06:15 -0400 |
commit | 723f90119512fa29b8b407d296e01219c4ead04f (patch) | |
tree | 38655e5435ef1dd1396810c10fb360014b09c2e2 /lib | |
parent | b71bc694c8a08804714a9469a48f58dac5dd96cb (diff) | |
download | curl-723f90119512fa29b8b407d296e01219c4ead04f.tar.gz |
http2: Improve header parsing
- Error if a header line is larger than supported.
- Warn if cumulative header line length may be larger than supported.
- Allow spaces when parsing the path component.
- Make sure each header line ends in \r\n. This fixes an out of bounds.
- Disallow header continuation lines until we decide what to do.
Ref: https://github.com/curl/curl/issues/659
Ref: https://github.com/curl/curl/pull/663
Diffstat (limited to 'lib')
-rw-r--r-- | lib/http2.c | 132 |
1 files changed, 102 insertions, 30 deletions
diff --git a/lib/http2.c b/lib/http2.c index f9e873ac6..1f4c6ffcc 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -35,6 +35,7 @@ #include "conncache.h" #include "url.h" #include "connect.h" +#include "strtoofft.h" /* The last #include files should be: */ #include "curl_memory.h" @@ -1491,6 +1492,9 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, field list. */ #define AUTHORITY_DST_IDX 3 +#define HEADER_OVERFLOW(x) \ + (x.namelen > (uint16_t)-1 || x.valuelen > (uint16_t)-1 - x.namelen) + /* return number of received (decrypted) bytes */ static ssize_t http2_send(struct connectdata *conn, int sockindex, const void *mem, size_t len, CURLcode *err) @@ -1503,12 +1507,12 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, int rv; struct http_conn *httpc = &conn->proto.httpc; struct HTTP *stream = conn->data->req.protop; - nghttp2_nv *nva; + nghttp2_nv *nva = NULL; size_t nheader; size_t i; size_t authority_idx; char *hdbuf = (char*)mem; - char *end; + char *end, *line_end; nghttp2_data_provider data_prd; int32_t stream_id; nghttp2_session *h2 = httpc->h2; @@ -1559,12 +1563,16 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, /* Here, we assume the curl http code generate *correct* HTTP header field block */ nheader = 0; - for(i = 0; i < len; ++i) { - if(hdbuf[i] == 0x0a) { + for(i = 1; i < len; ++i) { + if(hdbuf[i] == '\n' && hdbuf[i - 1] == '\r') { ++nheader; + ++i; } } - /* We counted additional 2 \n in the first and last line. We need 3 + if(nheader < 2) + goto fail; + + /* We counted additional 2 \r\n in the first and last line. We need 3 new headers: :method, :path and :scheme. Therefore we need one more space. */ nheader += 1; @@ -1573,51 +1581,84 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, *err = CURLE_OUT_OF_MEMORY; return -1; } + /* Extract :method, :path from request line */ - end = strchr(hdbuf, ' '); - if(!end) + line_end = strstr(hdbuf, "\r\n"); + + /* Method does not contain spaces */ + end = memchr(hdbuf, ' ', line_end - hdbuf); + if(!end || end == hdbuf) goto fail; nva[0].name = (unsigned char *)":method"; - nva[0].namelen = (uint16_t)strlen((char *)nva[0].name); + nva[0].namelen = strlen((char *)nva[0].name); nva[0].value = (unsigned char *)hdbuf; - nva[0].valuelen = (uint16_t)(end - hdbuf); + nva[0].valuelen = (size_t)(end - hdbuf); nva[0].flags = NGHTTP2_NV_FLAG_NONE; + if(HEADER_OVERFLOW(nva[0])) { + failf(conn->data, "Failed sending HTTP request: Header overflow"); + goto fail; + } hdbuf = end + 1; - end = strchr(hdbuf, ' '); - if(!end) + /* Path may contain spaces so scan backwards */ + end = NULL; + for(i = (size_t)(line_end - hdbuf); i; --i) { + if(hdbuf[i - 1] == ' ') { + end = &hdbuf[i - 1]; + break; + } + } + if(!end || end == hdbuf) goto fail; nva[1].name = (unsigned char *)":path"; - nva[1].namelen = (uint16_t)strlen((char *)nva[1].name); + nva[1].namelen = strlen((char *)nva[1].name); nva[1].value = (unsigned char *)hdbuf; - nva[1].valuelen = (uint16_t)(end - hdbuf); + nva[1].valuelen = (size_t)(end - hdbuf); nva[1].flags = NGHTTP2_NV_FLAG_NONE; + if(HEADER_OVERFLOW(nva[1])) { + failf(conn->data, "Failed sending HTTP request: Header overflow"); + goto fail; + } + hdbuf = end + 1; + + end = line_end; nva[2].name = (unsigned char *)":scheme"; - nva[2].namelen = (uint16_t)strlen((char *)nva[2].name); + nva[2].namelen = strlen((char *)nva[2].name); if(conn->handler->flags & PROTOPT_SSL) nva[2].value = (unsigned char *)"https"; else nva[2].value = (unsigned char *)"http"; - nva[2].valuelen = (uint16_t)strlen((char *)nva[2].value); + nva[2].valuelen = strlen((char *)nva[2].value); nva[2].flags = NGHTTP2_NV_FLAG_NONE; - - hdbuf = strchr(hdbuf, 0x0a); - if(!hdbuf) + if(HEADER_OVERFLOW(nva[2])) { + failf(conn->data, "Failed sending HTTP request: Header overflow"); goto fail; - ++hdbuf; + } authority_idx = 0; - i = 3; while(i < nheader) { size_t hlen; int skip = 0; - end = strchr(hdbuf, ':'); - if(!end) + + hdbuf = line_end + 2; + + line_end = strstr(hdbuf, "\r\n"); + if(line_end == hdbuf) + goto fail; + + /* header continuation lines are not supported */ + if(*hdbuf == ' ' || *hdbuf == '\t') + goto fail; + + for(end = hdbuf; end < line_end && *end != ':'; ++end) + ; + if(end == hdbuf || end == line_end) goto fail; hlen = end - hdbuf; + if(hlen == 10 && Curl_raw_nequal("connection", hdbuf, 10)) { /* skip Connection: headers! */ skip = 1; @@ -1626,21 +1667,24 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, else if(hlen == 4 && Curl_raw_nequal("host", hdbuf, 4)) { authority_idx = i; nva[i].name = (unsigned char *)":authority"; - nva[i].namelen = (uint16_t)strlen((char *)nva[i].name); + nva[i].namelen = strlen((char *)nva[i].name); } else { nva[i].name = (unsigned char *)hdbuf; - nva[i].namelen = (uint16_t)(end - hdbuf); + nva[i].namelen = (size_t)(end - hdbuf); } hdbuf = end + 1; - for(; *hdbuf == ' '; ++hdbuf); - end = strchr(hdbuf, 0x0d); - if(!end) - goto fail; + while(*hdbuf == ' ' || *hdbuf == '\t') + ++hdbuf; + end = line_end; if(!skip) { nva[i].value = (unsigned char *)hdbuf; - nva[i].valuelen = (uint16_t)(end - hdbuf); + nva[i].valuelen = (size_t)(end - hdbuf); nva[i].flags = NGHTTP2_NV_FLAG_NONE; + if(HEADER_OVERFLOW(nva[i])) { + failf(conn->data, "Failed sending HTTP request: Header overflow"); + goto fail; + } /* Inspect Content-Length header field and retrieve the request entity length so that we can set END_STREAM to the last DATA frame. */ @@ -1648,7 +1692,13 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, Curl_raw_nequal("content-length", (char*)nva[i].name, 14)) { size_t j; stream->upload_left = 0; + if(!nva[i].valuelen) + goto fail; for(j = 0; j < nva[i].valuelen; ++j) { + if(nva[i].value[j] < '0' || nva[i].value[j] > '9') + goto fail; + if(stream->upload_left >= CURL_OFF_T_MAX / 10) + goto fail; stream->upload_left *= 10; stream->upload_left += nva[i].value[j] - '0'; } @@ -1659,7 +1709,6 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, } ++i; } - hdbuf = end + 2; } /* :authority must come before non-pseudo header fields */ @@ -1671,6 +1720,29 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, nva[i] = authority; } + /* Warn stream may be rejected if cumulative length of headers is too large. + It appears nghttp2 will not send a header frame larger than 64KB. */ + { + size_t acc = 0; + const size_t max_acc = 60000; /* <64KB to account for some overhead */ + + for(i = 0; i < nheader; ++i) { + if(nva[i].namelen > max_acc - acc) + break; + acc += nva[i].namelen; + + if(nva[i].valuelen > max_acc - acc) + break; + acc += nva[i].valuelen; + } + + if(i != nheader) { + infof(conn->data, "http2_send: Warning: The cumulative length of all " + "headers exceeds %zu bytes and that could cause the " + "stream to be rejected.\n", max_acc); + } + } + h2_pri_spec(conn->data, &pri_spec); switch(conn->data->set.httpreq) { |