镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Commit df4011d

Browse files
mcollinaaduh95
authored andcommitted
http2: guard against null write request on cancelled stream
Cancelling an in-flight `respondWithFile()` download can leave a file read pending on the thread pool after the HTTP/2 stream has been destroyed. When that read completes, `StreamPipe` forwards a null `WriteWrap` to the previous listener, which dereferences it in `ReportWritesToJSStreamListener::OnStreamAfterReqFinished()`, crashing the server with a NULL pointer dereference (segfault). Guard against a null stream and null request in `OnStreamAfterReqFinished()` so the listener returns early. Refs: https://hackerone.com/4150894 Refs: #64623 Signed-off-by: Matteo Collina <hello@matteocollina.com> Assisted-by: pi PR-URL: #67252 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> CVE-ID: CVE-2026-123875
1 parent 458a0a8 commit df4011d

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

‎src/stream_base.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,8 @@ void CustomBufferJSListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
804804
void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
805805
StreamReq* req_wrap, int status) {
806806
StreamBase* stream = static_cast<StreamBase*>(stream_);
807+
if (stream == nullptr) return;
808+
if (req_wrap == nullptr) return;
807809
Environment* env = stream->stream_env();
808810
if (!env->can_call_into_js()) return;
809811
AsyncWrap* async_wrap = req_wrap->GetAsyncWrap();
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
3+
// Regression test for https://hackerone.com/4150894 (NULL dereference in
4+
// StreamPipe). Cancelling an in-flight `respondWithFile()` download while a
5+
// file read is still pending on the thread pool crashed the server: a NULL
6+
// WriteWrap was forwarded to ReportWritesToJSStreamListener, which then
7+
// dereferenced it in OnStreamAfterReqFinished().
8+
9+
const common = require('../common');
10+
if (!common.hasCrypto)
11+
common.skip('missing crypto');
12+
const http2 = require('http2');
13+
14+
const server = http2.createServer();
15+
server.on('stream', (stream) => {
16+
stream.on('error', () => {});
17+
// Use a file large enough that a read is still in flight when the client
18+
// destroys the session.
19+
stream.respondWithFile(process.execPath);
20+
});
21+
server.on('sessionError', () => {});
22+
23+
server.listen(0, common.mustCall(() => {
24+
const port = server.address().port;
25+
const rounds = 50;
26+
let i = 0;
27+
let done = false;
28+
29+
function cancelOne() {
30+
if (done) return;
31+
if (i >= rounds) {
32+
done = true;
33+
server.close(common.mustCall());
34+
return;
35+
}
36+
i++;
37+
const client = http2.connect(`http://127.0.0.1:${port}`);
38+
client.on('error', () => {});
39+
const req = client.request({ ':path': '/' });
40+
req.on('error', () => {});
41+
req.resume();
42+
// Destroy the session shortly after the request is sent, while the server
43+
// may still be reading the file. Vary the delay to hit the race window.
44+
setTimeout(() => {
45+
client.destroy();
46+
setTimeout(cancelOne, 1);
47+
}, 3 + (i * 7) % 40);
48+
}
49+
50+
cancelOne();
51+
}));

0 commit comments

Comments
 (0)