Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, void@tarantool.org,
	lvasiliev@tarantool.org
Subject: [Tarantool-patches] [PATCH v1] http_parser: fix UB triggered by using negative array index
Date: Sat, 13 Feb 2021 20:43:47 +0300	[thread overview]
Message-ID: <9b993ed9540d52ccd07f3c0e675f545801874a11.1613237530.git.sergeyb@tarantool.org> (raw)

From: Sergey Bronnikov <sergeyb@tarantool.org>

http_parser() function resolves symbol to lower case using array, where
an char code maps to an appropriate symbol. Some symbols may have
negative ASCII code, but array index cannot be negative, it is an
undefined behaviour.

I read parts relevant to message headers in "Hypertext Transfer Protocol
(HTTP/1.1): Message Syntax and Routing" [1] and "Hypertext Transfer
Protocol -- HTTP/1.1" [2] and found that only ASCII symbols only allowed
in message headers. It means that symbols with negative ASCII code are
not allowed. Patch adds a check for each processed symbol to have
positive ASCII code, otherwise function returns HTTP_PARSE_INVALID.

1. RFC 7230 - 3.2 Header Fields, https://tools.ietf.org/html/rfc7230#section-3.2
2. RFC 2616 - Message Headers, https://www.ietf.org/rfc/rfc2616.txt

Bug found using LibFuzzer combined with UBsan

Closes https://github.com/tarantool/security/issues/6
Needed for: https://github.com/google/oss-fuzz/pull/4723
---
Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/255857504
Github CI: https://github.com/tarantool/tarantool/commit/9b993ed9540d52ccd07f3c0e675f545801874a11
Issue: https://github.com/tarantool/security/issues/6
Branch: ligurio/http_parser_crash

 src/lib/http_parser/http_parser.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/lib/http_parser/http_parser.c b/src/lib/http_parser/http_parser.c
index 28093b79a..b9bee10f6 100644
--- a/src/lib/http_parser/http_parser.c
+++ b/src/lib/http_parser/http_parser.c
@@ -255,6 +255,9 @@ http_parse_header_line(struct http_parser *prsr, char **bufp,
 
 	for (; p < end_buf; p++) {
 		ch = *p;
+		if (ch < 0) {
+			return HTTP_PARSE_INVALID;
+		}
 		switch (state) {
 		/* first char */
 		case sw_start:
-- 
2.25.1

                 reply	other threads:[~2021-02-13 18:10 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b993ed9540d52ccd07f3c0e675f545801874a11.1613237530.git.sergeyb@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=lvasiliev@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=void@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v1] http_parser: fix UB triggered by using negative array index' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox