Skip to content

Commit 83128eb

Browse files
authored
fix: fixes response.attachment behavior leads to Content-Type Sniffing (#1904)
## Checklist Added security-focused tests to verify: 1. Content-Type is preserved when already set 2. Content-Type is still set when not previously defined (backwards compatibility) 3. The fix prevents XSS vulnerabilities with HTML and SVG files credit "Luca Carettoni of Doyensec LLC" as [requested in the advisory](https://github.com/koajs/koa/security/advisories/GHSA-c5vw-j4hf-j526).
1 parent 1ddb048 commit 83128eb

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

__tests__/response/attachment.test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,28 @@ const request = require('supertest')
77
const Koa = require('../..')
88

99
describe('ctx.attachment([filename])', () => {
10+
describe('security: prevent Content-Type override (GHSA-c5vw-j4hf-j526)', () => {
11+
it('should NOT override Content-Type when already set', () => {
12+
const ctx = context()
13+
ctx.response.set('Content-Type', 'application/octet-stream')
14+
ctx.attachment('malicious.html')
15+
assert.strictEqual(ctx.response.get('Content-Type'), 'application/octet-stream')
16+
assert.strictEqual(ctx.response.header['content-disposition'], 'attachment; filename="malicious.html"')
17+
})
18+
19+
it('should preserve safe Content-Type for SVG files', () => {
20+
const ctx = context()
21+
ctx.response.set('Content-Type', 'application/octet-stream')
22+
ctx.attachment('image.svg')
23+
assert.strictEqual(ctx.response.get('Content-Type'), 'application/octet-stream')
24+
})
25+
26+
it('should set Content-Type when not previously set', () => {
27+
const ctx = context()
28+
ctx.attachment('document.pdf')
29+
assert.strictEqual(ctx.response.get('Content-Type'), 'application/pdf')
30+
})
31+
})
1032
describe('when given a filename', () => {
1133
it('should set the filename param', () => {
1234
const ctx = context()

lib/response.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,9 @@ module.exports = {
351351
*/
352352

353353
attachment (filename, options) {
354-
if (filename) this.type = extname(filename)
354+
if (filename && !this.has('Content-Type')) {
355+
this.type = extname(filename)
356+
}
355357
this.set('Content-Disposition', contentDisposition(filename, options))
356358
},
357359

0 commit comments

Comments
 (0)