Skip to content

Commit cd8af90

Browse files
committed
fix(retry): validate resumed response framing
Checkpoint the original response framing before exposing headers and reject resumed responses whose Content-Range does not match it. Do not retry after an exposed response without a resumable length, preventing attacker-controlled retry responses from being concatenated into downstream output. Fixes: GHSA-r53p-7pc4-xj5r CVE: CVE-2026-18540 Signed-off-by: Matteo Collina <hello@matteocollina.com> (cherry picked from commit a424f8cc8fde7fd12cb1a09f84344335d5e469e6)
1 parent 6615e01 commit cd8af90

2 files changed

Lines changed: 217 additions & 3 deletions

File tree

‎lib/handler/retry-handler.js‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class RetryHandler {
9696
// Preserve old behavior for status codes that are not eligible for retry
9797
if (this.retryOpts.statusCodes.includes(statusCode) === false) {
9898
this.headersSent = true
99+
this.checkpointResponseEnd(headers)
99100
this.handler.onResponseStart?.(controller, statusCode, headers, statusMessage)
100101
} else {
101102
this.error = err
@@ -106,13 +107,15 @@ class RetryHandler {
106107

107108
if (isDisturbed(this.opts.body)) {
108109
this.headersSent = true
110+
this.checkpointResponseEnd(headers)
109111
this.handler.onResponseStart?.(controller, statusCode, headers, statusMessage)
110112
return
111113
}
112114

113115
function shouldRetry (passedErr) {
114116
if (passedErr) {
115117
this.headersSent = true
118+
this.checkpointResponseEnd(headers)
116119
this.handler.onResponseStart?.(controller, statusCode, headers, statusMessage)
117120
controller.resume()
118121
return
@@ -133,6 +136,20 @@ class RetryHandler {
133136
)
134137
}
135138

139+
checkpointResponseEnd (headers) {
140+
if (this.end == null && this.opts.method !== 'HEAD') {
141+
const contentLength = headers['content-length']
142+
this.end = contentLength != null ? Number(contentLength) - 1 : null
143+
144+
assert(
145+
this.end == null || Number.isFinite(this.end),
146+
'invalid content-length'
147+
)
148+
149+
this.resume = this.end != null
150+
}
151+
}
152+
136153
onRequestStart (controller, context) {
137154
if (!this.headersSent) {
138155
this.handler.onRequestStart?.(controller, context)
@@ -253,8 +270,12 @@ class RetryHandler {
253270

254271
const { start, size, end = size ? size - 1 : null } = contentRange
255272

256-
assert(this.start === start, 'content-range mismatch')
257-
assert(this.end == null || this.end === end, 'content-range mismatch')
273+
if (this.start !== start || (this.end != null && this.end !== end)) {
274+
throw new RequestRetryError('Content-Range mismatch', statusCode, {
275+
headers,
276+
data: { count: this.retryCount }
277+
})
278+
}
258279

259280
return
260281
}
@@ -379,7 +400,7 @@ class RetryHandler {
379400
}
380401

381402
onResponseError (controller, err) {
382-
if (controller?.aborted || isDisturbed(this.opts.body)) {
403+
if (controller?.aborted || isDisturbed(this.opts.body) || (this.headersSent && !this.resume)) {
383404
this.handler.onResponseError?.(controller, err)
384405
return
385406
}

‎test/interceptors/retry.js‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,199 @@ test('Should handle 206 partial content - bad-etag', async t => {
511511
}
512512
})
513513

514+
test('#4970 - Should reject resumed partial content when body exceeds Content-Range', async t => {
515+
t = tspl(t, { plan: 5 })
516+
517+
let x = 0
518+
const injectedResponse = 'HTTP/1.1 302 Found\r\nLocation: http://evil.com\r\nContent-Length: 0\r\n\r\n'
519+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
520+
if (x === 0) {
521+
t.ok(true, 'pass')
522+
res.setHeader('content-length', '5')
523+
res.setHeader('etag', '123')
524+
res.write('use')
525+
setTimeout(() => {
526+
res.destroy()
527+
}, 1e2)
528+
} else if (x === 1) {
529+
t.deepStrictEqual(req.headers.range, 'bytes=3-4')
530+
t.deepStrictEqual(req.headers['if-match'], '123')
531+
res.statusCode = 206
532+
res.setHeader('etag', '123')
533+
res.setHeader('content-range', 'bytes 3-4/5')
534+
res.end(`r1${injectedResponse}`)
535+
}
536+
x++
537+
})
538+
539+
const requestOptions = {
540+
method: 'GET',
541+
path: '/',
542+
headers: {
543+
'content-type': 'application/json'
544+
},
545+
retryOptions: {
546+
retry: (err, { state, opts }, done) => {
547+
if (err.message.includes('other side closed')) {
548+
setTimeout(done, 100)
549+
return
550+
}
551+
552+
return done(err)
553+
}
554+
}
555+
}
556+
557+
server.listen(0)
558+
559+
await once(server, 'listening')
560+
561+
const client = new Client(
562+
`http://localhost:${server.address().port}`
563+
).compose(retry())
564+
565+
after(async () => {
566+
await client.close()
567+
server.close()
568+
569+
await once(server, 'close')
570+
})
571+
572+
const response = await client.request(requestOptions)
573+
t.strictEqual(response.statusCode, 200)
574+
await t.rejects(response.body.text(), {
575+
name: 'RequestRetryError',
576+
code: 'UND_ERR_REQ_RETRY',
577+
message: 'Content-Length mismatch'
578+
})
579+
})
580+
581+
test('#3900615 - Should reject a resumed response that exceeds an error response content-length', async t => {
582+
t = tspl(t, { plan: 3 })
583+
584+
let x = 0
585+
let retries = 0
586+
const injectedResponse = 'HTTP/1.1 302 Found\r\nLocation: http://evil.com\r\nContent-Length: 0\r\n\r\n'
587+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
588+
if (x === 0) {
589+
res.statusCode = 404
590+
res.setHeader('content-length', '2')
591+
res.end('1', () => res.destroy())
592+
} else if (x === 1) {
593+
t.strictEqual(req.headers.range, 'bytes=1-1')
594+
res.statusCode = 206
595+
res.setHeader('connection', 'close')
596+
res.setHeader('content-range', `bytes 1-${injectedResponse.length + 1}/${injectedResponse.length + 2}`)
597+
res.end(`2${injectedResponse}`)
598+
}
599+
x++
600+
})
601+
602+
server.listen(0)
603+
604+
await once(server, 'listening')
605+
606+
const client = new Client(
607+
`http://localhost:${server.address().port}`
608+
).compose(retry())
609+
610+
after(async () => {
611+
await client.destroy()
612+
server.closeAllConnections()
613+
server.close()
614+
615+
await once(server, 'close')
616+
})
617+
618+
const response = await client.request({
619+
method: 'GET',
620+
path: '/',
621+
retryOptions: {
622+
retry: (err, _context, done) => {
623+
if (err.message.includes('other side closed') && retries++ === 0) {
624+
done(null)
625+
return
626+
}
627+
628+
done(err)
629+
}
630+
}
631+
})
632+
t.strictEqual(response.statusCode, 404)
633+
await t.rejects(response.body.text(), {
634+
name: 'RequestRetryError',
635+
code: 'UND_ERR_REQ_RETRY',
636+
message: 'Content-Range mismatch'
637+
})
638+
})
639+
640+
test('#3900104 - Should not resume a 206 response without a usable content-range', async t => {
641+
t = tspl(t, { plan: 4 })
642+
643+
let x = 0
644+
const server = createServer({ joinDuplicateHeaders: true }, (_req, res) => {
645+
t.strictEqual(x, 0, 'must not retry an uncheckpointed partial response')
646+
res.statusCode = 206
647+
res.setHeader('content-length', '2')
648+
res.setHeader('content-range', 'bytes 0-999')
649+
res.write('1', () => res.destroy())
650+
x++
651+
})
652+
653+
server.listen(0)
654+
655+
await once(server, 'listening')
656+
657+
const client = new Client(
658+
`http://localhost:${server.address().port}`
659+
).compose(retry())
660+
661+
after(async () => {
662+
await client.destroy()
663+
server.close()
664+
665+
await once(server, 'close')
666+
})
667+
668+
const response = await client.request({ method: 'GET', path: '/' })
669+
t.strictEqual(response.statusCode, 206)
670+
await t.rejects(response.body.text(), {
671+
name: 'SocketError',
672+
code: 'UND_ERR_SOCKET',
673+
message: 'other side closed'
674+
})
675+
t.strictEqual(x, 1)
676+
})
677+
678+
test('Should not reject a HEAD response with content-length', async t => {
679+
t = tspl(t, { plan: 3 })
680+
681+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
682+
res.setHeader('content-length', '1234')
683+
res.end()
684+
})
685+
686+
server.listen(0)
687+
688+
await once(server, 'listening')
689+
690+
const client = new Client(
691+
`http://localhost:${server.address().port}`
692+
).compose(retry())
693+
694+
after(async () => {
695+
await client.close()
696+
server.close()
697+
698+
await once(server, 'close')
699+
})
700+
701+
const response = await client.request({ method: 'HEAD', path: '/' })
702+
t.strictEqual(response.statusCode, 200)
703+
t.strictEqual(response.headers['content-length'], '1234')
704+
t.strictEqual(await response.body.text(), '')
705+
})
706+
514707
test('retrying a request with a body', async t => {
515708
t = tspl(t, { plan: 2 })
516709
let counter = 0

0 commit comments

Comments
 (0)