Node.js: Vulnerability in http-parser & embedded NULL header handling

Due to a snafu in how [email protected] is setup to forward (see https://github.com/envoyproxy/envoy/issues/5155), the following bug report was not made available prior to disclosure. For completeness, I’m providing the original e-mail below.

Please note that this has been fixed in http-parser since disclosures. I understand that Node has moved away from http-parser, but this might affect Node.JS LTS for earlier versions. See https://github.com/nodejs/http-parser/issues/468 for the fix.

Rather than file a full report, I would like to share with Node.JS security WG the following resources:

• Envoy CVE: https://vulners.com/cve/CVE-2019-9900
• Envoy GH issue with CVE details: https://github.com/envoyproxy/envoy/issues/6434
• http-parser GH issue: https://github.com/nodejs/http-parser/issues/468
• Other discussion of the handling of this issue: https://github.com/envoyproxy/envoy/issues/5155#issuecomment-481854258

Original e-mail

MIME-Version: 1.0
Date: Thu, 14 Mar 2019 16:35:20 -0400
Message-ID: <CAA4W8ZmaBzTMFU8VdpJzVDM7LXo0o5-WPTdYisGJUF9qsXiPnQ@mail.gmail.com>
Subject: Vulnerability in http-parser & embedded NULL header handling
From: Harvey Tuch <[email protected]>
To: [email protected], [email protected]
Cc: [email protected]
Content-Type: multipart/alternative; boundary=“0000000000006b3f64058413dc0b”

–0000000000006b3f64058413dc0b
Content-Type: text/plain; charset=“UTF-8”

Hi Node.js Security WG, Ryan,

We (Envoy security team) have discovered a potential security vulnerability
related to our use of http-parser that we are working to fix, patch and
issue a security update for Envoy-side following
https://github.com/envoyproxy/envoy/blob/master/SECURITY_RELEASE_PROCESS.md.

We would like to give you advanced notice of this under embargo, check in
with you to see if this might affect Node.js or other http-parser users,
and potentially coordinate on an http-parser side fix.

Envoy makes use of http-parser as its HTTP/1 codec on its data plane. Envoy
has baked into it today the assumption that its HTTP codecs (http-parser,
nghttp2) enforce RFC constraints on valid header values (
https://tools.ietf.org/html/rfc7230#section-3.2.6). In particular, we
expect that there are no embedded NULLs in header values or keys that are
placed in Envoy’s HeaderStrings and HeaderMapImpls objects. This is
particularly important because we allow two views of a HeaderString, via
c_str()
<https://github.com/envoyproxy/envoy/blob/b41ba5925a4e93d22a86c6501d63314ccf0d79f3/include/envoy/http/header_map.h#L115&gt;
and getStringView()
<https://github.com/envoyproxy/envoy/blob/b41ba5925a4e93d22a86c6501d63314ccf0d79f3/include/envoy/http/header_map.h#L120&gt;;
embedded NULLS cause inconsistent views and lengths through these
accessors. We use a mixture of these in header matching and routing.

Our fuzzers and some recently introduced ASSERTs indicated that embedded
NULLs were making their way into header values received from http-parser.
Digging deeper, the errant behavior is due to a bug in how validation of
header values is performed by http-parser. You can see this in the
validation logic at
https://github.com/nodejs/http-parser/blob/0d0a24e19eb5ba232d2ea8859aba2a7cc6c42bc4/http_parser.c#L1469
.

In particular, only the first character of the header value is validate at
line 1490. Then the entire header value is accepted via a memchr scan at
https://github.com/nodejs/http-parser/blob/0d0a24e19eb5ba232d2ea8859aba2a7cc6c42bc4/http_parser.c#L1506
.

This means that we can have arbitrary embedded NULLs in any HTTP/1.1 header
value today. There are places in Envoy where we use one view of these
strings for matching (e.g. route table lookup, authorization) and another
view for routing and sending to our upstreams.

We have scored this as 6.5 using CVSS and will work on an Envoy patch to
reject any header values that contain NULL and issue a point release and
public disclosure following
https://github.com/envoyproxy/envoy/blob/master/SECURITY_RELEASE_PROCESS.md
ASAP.

This issue can/should also be resolved in http-parser by ensuring the
entire header value is validated as per RFC. We do not plan on doing the
fix for this prior to our release, but will coordinate with you folks if
you are interested in doing so.

Please advise on how you would like to proceed with this. We are planning
to get our disclosure/release happening either end-of-week or early next
week, but we would like to provide you an opportunity to provide some input
here, since this has not been publicly disclosed yet we can provide some
additional time if needed.

Thanks,
Harvey (on behalf of Envoy security team)

–0000000000006b3f64058413dc0b
Content-Type: text/html; charset=“UTF-8”
Content-Transfer-Encoding: quoted-printable

<div><div><div>Hi Node.js Security WG, =
Ryan,<div><br></div><div>We (Envoy security team) have discovered a potenti=
al security vulnerability related to our use of http-parser that we are wor=
king to fix, patch and issue a security update for Envoy-side following=C2=
=A0<a href>https://github.com/envoyproxy/envoy/blob/master/SECURITY_RE=
LEASE_PROCESS.md</a>.</div><div><br></div><div>We would like to give you ad=
vanced notice of this under embargo, check in with you to see if this might=
affect Node.js or other http-parser users, and potentially coordinate on a=
n http-parser side fix.</div><div><br></div><div>Envoy makes use of http-pa=
rser as its HTTP/1 codec on its data plane. Envoy has baked into it today t=
he assumption that its HTTP codecs (http-parser, nghttp2) enforce RFC const=
raints on valid header values (<a href target=“3D"_blank"”>https://tools.ietf.org/html/rfc7230#sec=
tion-3.2.6</a>). In particular, we expect that there are no embedded NULLs =
in header values or keys that are placed in Envoy's HeaderStrings and H=
eaderMapImpls objects. This is particularly important because we allow two =
views of a HeaderString, via=C2=A0<a href target=“3D"_blank"”>c_str()</a>=C2=A0and=C2=A0<a href target=“3D"_blank"”>getStringView()</=
a>; embedded NULLS cause inconsistent views and lengths through these acces=
sors. We use a mixture of these in header matching and routing.</div></div>=
<div><br></div><div>Our fuzzers and some recently introduced ASSERTs indica=
ted that embedded NULLs were making their way into header values received f=
rom http-parser. Digging deeper, the errant behavior is due to a bug in how=
validation of header values is performed by http-parser. You can see this =
in the validation logic at=C2=A0<a href>https://github.com/nodejs/http-parser/blob/0d0a24e19eb5ba23=
2d2ea8859aba2a7cc6c42bc4/http_parser.c#L1469</a>.=C2=A0</div><div><br></div=
><div>In particular, only the first character of the header value is valida=
te at line 1490. Then the entire header value is accepted via a memchr scan=
at=C2=A0<a href target=“3D"_blank"”>https:/=
/github.com/nodejs/http-parser/blob/0d0a24e19eb5ba232d2ea8859aba2a7cc6c42bc=
4/http_parser.c#L1506</a>.</div><div><br></div><div>This means that we can =
have arbitrary embedded NULLs in any HTTP/1.1 header value today. There are=
places in Envoy where we use one view of these strings for matching (e.g. =
route table lookup, authorization) and another view for routing and sending=
to our upstreams.<br></div><div><br></div><div>We have scored this as 6.5 =
using CVSS and will work on an Envoy patch to reject any header values that=
contain NULL and issue a point release and public disclosure following=C2=
=A0<a href>https://github.com/envoyproxy/envoy/blob/master/SECURITY_RE=
LEASE_PROCESS.md</a> ASAP.</div><div><br></div><div>This issue can/should a=
lso be resolved in http-parser by ensuring the entire header value is valid=
ated as per RFC. We do not plan on doing the fix for this prior to our rele=
ase, but will coordinate with you folks if you are interested in doing so.<=
/div><div><br></div><div>Please advise on how you would like to proceed wit=
h this. We are planning to get our disclosure/release happening either end-=
of-week or early next week, but we would like to provide you an opportunity=
to provide some input here, since this has not been publicly disclosed yet=
we can provide some additional time if needed.</div><div><br></div><div>Th=
anks,</div><div>Harvey (on behalf of Envoy security team)</div><div><br></d=
iv></div></div>

–0000000000006b3f64058413dc0b–

Impact

This has a CVSS score of 8.3 for Envoy as a project consuming http-parser. The impact on Node.JS is unclear to me.