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:53 UTC
[incubator-nuttx-apps] branch master updated (d74ac65 -> 95c9007)
This is an automated email from the ASF dual-hosted git repository.
acassis pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx-apps.git.
from d74ac65 hdc1008_demo: set intial measurement mode
new f17af21 webclient: Add a few ninfo in header parsing
new eb1a99f webclient.c: Don't parse the status line as the first header line
new b533750 webclient.c: Fix buffer overrun in HTTP header parsing
new 95c9007 webclient: Fix buffer overrun in wget_parsestatus
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
netutils/webclient/webclient.c | 88 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 81 insertions(+), 7 deletions(-)
[incubator-nuttx-apps] 02/04: webclient.c: Don't parse the status
line as the first header line
Posted by ac...@apache.org.
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 eb1a99fdd6980c2366ab8afd7327c38941438d08
Author: YAMAMOTO Takashi <ya...@midokura.com>
AuthorDate: Fri May 14 10:57:57 2021 +0900
webclient.c: Don't parse the status line as the first header line
---
netutils/webclient/webclient.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/netutils/webclient/webclient.c b/netutils/webclient/webclient.c
index d48196d..40c6951 100644
--- a/netutils/webclient/webclient.c
+++ b/netutils/webclient/webclient.c
@@ -447,6 +447,7 @@ static inline int wget_parsestatus(struct webclient_context *ctx,
*/
ws->state = WEBCLIENT_STATE_HEADERS;
+ ndx = 0;
break;
}
else
[incubator-nuttx-apps] 03/04: webclient.c: Fix buffer overrun in
HTTP header parsing
Posted by ac...@apache.org.
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
{
[incubator-nuttx-apps] 01/04: webclient: Add a few ninfo in header
parsing
Posted by ac...@apache.org.
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 f17af21f9c36a56e79ca26028a2857bea2578f73
Author: YAMAMOTO Takashi <ya...@midokura.com>
AuthorDate: Fri May 14 12:19:24 2021 +0900
webclient: Add a few ninfo in header parsing
---
netutils/webclient/webclient.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/netutils/webclient/webclient.c b/netutils/webclient/webclient.c
index e8c50d7..d48196d 100644
--- a/netutils/webclient/webclient.c
+++ b/netutils/webclient/webclient.c
@@ -406,6 +406,7 @@ static inline int wget_parsestatus(struct webclient_context *ctx,
}
ctx->http_status = http_status;
+ ninfo("Got HTTP status %lu\n", http_status);
if (ctx->http_reason != NULL)
{
strncpy(ctx->http_reason,
@@ -525,6 +526,7 @@ static inline int wget_parseheaders(struct wget_s *ws)
if (ndx > 0) /* Should always be true */
{
+ ninfo("Got HTTP header line: %.*s\n", ndx - 1, &ws->line[0]);
if (ws->line[0] == ISO_CR)
{
/* This was the last header line (i.e., and empty "\r\n"),
[incubator-nuttx-apps] 04/04: webclient: Fix buffer overrun in
wget_parsestatus
Posted by ac...@apache.org.
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 95c90076683d0f8e68cd53cc9f62acc40f4477a6
Author: YAMAMOTO Takashi <ya...@midokura.com>
AuthorDate: Fri May 14 11:24:58 2021 +0900
webclient: Fix buffer overrun in wget_parsestatus
Similarly to the fix in wget_parseheaders.
But simply always bail out as i guess it's very rare to see
that long status line.
Tested with an aritifically small CONFIG_WEBCLIENT_MAXHTTPLINE=20,
which is smaller than "HTTP/1.1 301 Moved Permanently".
---
netutils/webclient/webclient.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/netutils/webclient/webclient.c b/netutils/webclient/webclient.c
index 7f84ccc..72f5585 100644
--- a/netutils/webclient/webclient.c
+++ b/netutils/webclient/webclient.c
@@ -376,9 +376,20 @@ static inline int wget_parsestatus(struct webclient_context *ctx,
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)
{
+ if (!got_nl)
+ {
+ nerr("ERROR: HTTP status line didn't fit "
+ "CONFIG_WEBCLIENT_MAXHTTPLINE: %.*s\n",
+ ndx, ws->line);
+ return -E2BIG;
+ }
+
ws->line[ndx] = '\0';
if ((strncmp(ws->line, g_http10, strlen(g_http10)) == 0) ||
(strncmp(ws->line, g_http11, strlen(g_http11)) == 0))