sergiotarxz/nuttx-apps
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

webclient.c: Fix buffer overrun in HTTP header parsing

Browse Source 
* Detect a long header line which doesn't fit the buffer.

* If the header line in question doesn't seem important for us,
  just ignore it.

* Otherwise, bail out with -E2BIG.

The overrun was found by LLVM UBSan on macOS.

The following is an example of a long header line,
which doesn't fit the default CONFIG_WEBCLIENT_MAXHTTPLINE=200.
```
pacetanuki% curl -v https://www.midokura.com
:
:
< HTTP/2 200
< server: nginx
< date: Fri, 14 May 2021 02:16:24 GMT
< content-type: text/html; charset=UTF-8
< content-length: 131313
< x-powered-by: PHP/7.4.18
< link: <https://www.midokura.com/wp-json/>; rel="https://api.w.org/", <https://www.midokura.com/wp-json/wp/v2/pages/7>; rel="alternate"; type="application/json", <https://www.midokura.com/>; rel=shortlink
< x-powered-by: PleskLin
<
```
This commit is contained in:
YAMAMOTO Takashi 2021-05-14 11:09:32 +09:00 committed by Alan Carvalho de Assis
parent eb1a99fdd6
commit b53375074b
1 changed files with 67 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
netutils/webclient/webclient.c
View File

@ -152,6 +152,7 @@ struct wget_s
char line[CONFIG_WEBCLIENT_MAXHTTPLINE];
int ndx;
bool skip_to_next_line;
#ifdef CONFIG_WEBCLIENT_GETMIMETYPE
char mimetype[CONFIG_WEBCLIENT_MAXMIMESIZE];
@ -518,16 +519,35 @@ static inline int wget_parseheaders(struct wget_s *ws)
while (offset < ws->datend)
{
bool got_nl;
ws->line[ndx] = ws->buffer[offset];
if (ws->line[ndx] == ISO_NL)
got_nl = ws->line[ndx] == ISO_NL;
if (got_nl || ndx == CONFIG_WEBCLIENT_MAXHTTPLINE - 1)
{
/* We have an entire HTTP header line in s.line, so
* we parse it.
bool found;
if (ws->skip_to_next_line)
{
if (got_nl)
{
ws->skip_to_next_line = false;
}
ndx = 0;
continue;
}
/* We have an entire HTTP header line in ws->line, or
* our buffer is already full, so we start parsing it.
*/
found = false;
if (ndx > 0) /* Should always be true */
{
ninfo("Got HTTP header line: %.*s\n", ndx - 1, &ws->line[0]);
ninfo("Got HTTP header line%s: %.*s\n",
got_nl ? "" : " (truncated)",
ndx - 1, &ws->line[0]);
if (ws->line[0] == ISO_CR)
{
/* This was the last header line (i.e., and empty "\r\n"),
@ -541,7 +561,30 @@ static inline int wget_parseheaders(struct wget_s *ws)
/* Truncate the trailing \r\n */
ws->line[ndx - 1] = '\0';
if (got_nl)
{
ndx--;
if (ws->line[ndx] != ISO_CR)
{
nerr("ERROR: unexpected EOL from the server\n");
return -EPROTO;
}
}
ws->line[ndx] = '\0';
if (!strchr(ws->line, ':'))
{
if (got_nl)
{
nerr("ERROR: invalid header possibly due to "
"small CONFIG_WEBCLIENT_MAXHTTPLINE\n");
return -E2BIG;
}
nerr("ERROR: invalid header\n");
return -EPROTO;
}
/* Check for specific HTTP header fields. */
@ -559,6 +602,7 @@ static inline int wget_parseheaders(struct wget_s *ws)
strncpy(ws->mimetype, ws->line + strlen(g_httpcontenttype),
sizeof(ws->mimetype));
found = true;
}
else
#endif
@ -573,14 +617,30 @@ static inline int wget_parseheaders(struct wget_s *ws)
ret = parseurl(ws->line + strlen(g_httplocation), ws);
ninfo("New hostname='%s' filename='%s'\n",
ws->hostname, ws->filename);
found = true;
}
}
/* We're done parsing this line, so we reset the index to the start
* of the next line.
if (found && !got_nl)
{
/* We found something we might care.
* but we couldn't process it correctly.
*/
nerr("ERROR: truncated a header due to "
"small CONFIG_WEBCLIENT_MAXHTTPLINE\n");
return -E2BIG;
}
/* We're done parsing ws->line, so we reset the index.
*
* If we haven't seen the entire line yet (!got_nl),
* skip the rest of the line.
* Otherwise, start processing the next line.
*/
ndx = 0;
ws->skip_to_next_line = !got_nl;
}
else
{