Skip to content

Commit 8dff42c

Browse files
committed
[Fix] escape CR, LF, and " in field names and filenames
A field name or filename containing CRLF (or a `"`) was concatenated verbatim into the `Content-Disposition` header, letting a caller that passes attacker-controlled names break out of the header line to inject additional headers or smuggle extra multipart parts (CWE-93). Escape `\r`, `\n`, and `"` as `%0D`, `%0A`, and `%22`, matching the WHATWG HTML multipart/form-data encoding browsers use. Ref: GHSA-hmw2-7cc7-3qxx
1 parent 67b0f65 commit 8dff42c

2 files changed

Lines changed: 65 additions & 2 deletions

File tree

‎lib/form_data.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ var setToStringTag = require('es-set-tostringtag');
1515
var hasOwn = require('hasown');
1616
var populate = require('./populate.js');
1717

18+
/**
19+
* Escape CR, LF, and `"` in a multipart `name`/`filename` parameter, so a field
20+
* name or filename can not break out of its header line to inject headers or
21+
* smuggle additional parts. Matches the WHATWG HTML multipart/form-data encoding.
22+
*
23+
* @param {string} str - the parameter value to escape
24+
* @returns {string} the escaped value
25+
*/
26+
function escapeHeaderParam(str) {
27+
return String(str).replace(/\r/g, '%0D').replace(/\n/g, '%0A').replace(/"/g, '%22');
28+
}
29+
1830
/**
1931
* Create readable "multipart/form-data" streams.
2032
* Can be used to submit forms
@@ -180,7 +192,7 @@ FormData.prototype._multiPartHeader = function (field, value, options) {
180192
var contents = '';
181193
var headers = {
182194
// add custom disposition as third element or keep it two elements if not
183-
'Content-Disposition': ['form-data', 'name="' + field + '"'].concat(contentDisposition || []),
195+
'Content-Disposition': ['form-data', 'name="' + escapeHeaderParam(field) + '"'].concat(contentDisposition || []),
184196
// if no content type. allow it to be empty array
185197
'Content-Type': [].concat(contentType || [])
186198
};
@@ -234,7 +246,7 @@ FormData.prototype._getContentDisposition = function (value, options) { // eslin
234246
}
235247

236248
if (filename) {
237-
return 'filename="' + filename + '"';
249+
return 'filename="' + escapeHeaderParam(filename) + '"';
238250
}
239251
};
240252

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
3+
var common = require('../common');
4+
var assert = common.assert;
5+
6+
var FormData = require(common.dir.lib + '/form_data');
7+
8+
(function testFieldNameCRLFInjection() {
9+
var form = new FormData();
10+
form.append('email"\r\nX-Injected: true\r\nfake="', 'user@example.com');
11+
12+
var output = form.getBuffer().toString();
13+
14+
assert.equal(output.indexOf('X-Injected: true\r\n'), -1, 'CRLF in a field name must not produce an injected header line');
15+
assert.notEqual(output.indexOf('%0D%0A'), -1, 'CR/LF in a field name must be escaped');
16+
assert.notEqual(output.indexOf('%22'), -1, 'a `"` in a field name must be escaped');
17+
}());
18+
19+
(function testFilenameCRLFInjection() {
20+
var form = new FormData();
21+
form.append('file', Buffer.from('x'), { filename: 'a"\r\nX-Injected: yes\r\nb"' });
22+
23+
var output = form.getBuffer().toString();
24+
25+
assert.equal(output.indexOf('X-Injected: yes\r\n'), -1, 'CRLF in a filename must not produce an injected header line');
26+
assert.notEqual(output.indexOf('filename="a%22%0D%0A'), -1, 'CR/LF/`"` in a filename must be escaped');
27+
}());
28+
29+
(function testFieldNameBoundarySmuggling() {
30+
var form = new FormData();
31+
var boundary = form.getBoundary();
32+
33+
form.append('username"\r\n\r\nlegit_user\r\n--' + boundary + '\r\nContent-Disposition: form-data; name="is_admin"\r\n\r\ntrue\r\n--' + boundary + '\r\nContent-Disposition: form-data; name="fake', 'ignored_value');
34+
35+
var output = form.getBuffer().toString();
36+
var headerLines = output.split('\r\n').filter(function (line) {
37+
return line.indexOf('Content-Disposition') === 0;
38+
});
39+
40+
assert.equal(output.indexOf('name="is_admin"'), -1, 'a CRLF-laden field name must not smuggle an extra part');
41+
assert.equal(headerLines.length, 1, 'the whole field name must stay on one header line, smuggling no extra parts');
42+
}());
43+
44+
(function testOrdinaryFieldNamePreserved() {
45+
var form = new FormData();
46+
form.append('items[0]', 'value');
47+
48+
var output = form.getBuffer().toString();
49+
50+
assert.notEqual(output.indexOf('name="items[0]"'), -1, 'a field name without control characters must be unchanged');
51+
}());

0 commit comments

Comments
 (0)