While investigating whether envoyproxy/envoy#6434 applies to nghttp2 (it doesn't), I came across a situation where nghttp2 is invoking the nghttp2_session_callbacks_set_on_header_callback callback after a stream has closed and without input validation. This doesn't seem a security issue but is instead a functional issue.
What's happening is that a stream is closed when we get to
|
if (subject_stream && session_enforce_http_messaging(session)) { |
, so
subject_stream is null, but a header still is emitted at
|
rv = session_call_on_header(session, frame, &nv); |
.
Independently, Envoy had hit this case previously in mystery crashes, see https://github.com/envoyproxy/envoy/blob/48082bcd22fe9165eb73bed6d27857f578df63b5/source/common/http/http2/codec_impl.cc#L647.
This was also spotted by a fuzzer at https://oss-fuzz.com/testcase-detail/5765092623253504. However, this report is still private and doesn't have much non-Envoy specific useful information to share.
It seems the real issue is that we get headers emitted on closed streams; the lack of sanitization is just a side effect.
CC @mattklein123
While investigating whether envoyproxy/envoy#6434 applies to nghttp2 (it doesn't), I came across a situation where nghttp2 is invoking the
nghttp2_session_callbacks_set_on_header_callbackcallback after a stream has closed and without input validation. This doesn't seem a security issue but is instead a functional issue.What's happening is that a stream is closed when we get to
nghttp2/lib/nghttp2_session.c
Line 3622 in ec519f2
subject_streamis null, but a header still is emitted atnghttp2/lib/nghttp2_session.c
Line 3682 in ec519f2
Independently, Envoy had hit this case previously in mystery crashes, see https://github.com/envoyproxy/envoy/blob/48082bcd22fe9165eb73bed6d27857f578df63b5/source/common/http/http2/codec_impl.cc#L647.
This was also spotted by a fuzzer at https://oss-fuzz.com/testcase-detail/5765092623253504. However, this report is still private and doesn't have much non-Envoy specific useful information to share.
It seems the real issue is that we get headers emitted on closed streams; the lack of sanitization is just a side effect.
CC @mattklein123