You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ac...@apache.org on 2021/05/16 14:32:56 UTC
[incubator-nuttx-apps] 03/04: webclient.c: Fix buffer overrun in
HTTP header parsing
This is an automated email from the ASF dual-hosted git repository.
acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx-apps.git
commit b53375074b5fa17a7270cb41c8a44ae0af029907
Author: YAMAMOTO Takashi <ya...@midokura.com>
AuthorDate: Fri May 14 11:09:32 2021 +0900
webclient.c: Fix buffer overrun in HTTP header parsing
* 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
<
```
---
netutils/webclient/webclient.c | 74 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 67 insertions(+), 7 deletions(-)
diff --git a/netutils/webclient/webclient.c b/netutils/webclient/webclient.c
index 40c6951..7f84ccc 100644
--- a/netutils/webclient/webclient.c
+++ b/netutils/webclient/webclient.c
@@ -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
{