Skip to content

Commit 15cf6d1

Browse files
andris9claude
andcommitted
fix(mime-node): keep control chars out of header values and msg-id headers
A header field carries VCHAR and WSP only, but every C0 control character and DEL went out raw in Subject, in any header set through the headers option, in Message-ID, In-Reply-To, Content-Id and References, and in a List-* url. CR and LF were already stripped, so nothing crossed a header boundary, but the emitted header was not one an rfc5322 parser accepts. The treatment differs by grammar. An unstructured value is forced into the mime encoded word that a non-ascii value would get anyway, which is what the new _encodeHeaderText does. A msg-id and a url are structured, an encoded word inside the angle brackets would be read back as literal text, so there the characters are dropped instead. HT is left alone throughout. It is valid folding whitespace in a header value, and it separates the ids of an unfolded References header, so stripping it merged them into one unusable token. Stripping can make a scheme match that did not before, eg ht<NUL>tps://host now resolves to https://host. The allowlist in _formatListUrl is unchanged and still prefixes anything outside https, http, mailto and ftp, so a caller can not reach a scheme this way that a plain url could not already reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bYRfSN3MWR2St9WLuPeXA
1 parent 36bcf1a commit 15cf6d1

4 files changed

Lines changed: 96 additions & 5 deletions

File tree

lib/mailer/mail-message.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,9 @@ class MailMessage {
310310
}
311311

312312
_formatListUrl(url) {
313-
url = url.replace(/[\s<]+|[\s>]+/g, '');
313+
// a url has no way to carry a control char or DEL, and the angle brackets around it
314+
// are not a quoting construct, so anything left here lands in the header raw
315+
url = url.replace(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/g, '').replace(/[\s<]+|[\s>]+/g, '');
314316
if (/^(https?|mailto|ftp):/.test(url)) {
315317
return '<' + url + '>';
316318
}

lib/mime-node/index.js

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,7 +1121,13 @@ class MimeNode {
11211121
case 'Message-ID':
11221122
case 'In-Reply-To':
11231123
case 'Content-Id':
1124-
value = (value || '').toString().replace(/\r?\n|\r/g, ' ');
1124+
// a msg-id is structured, so an encoded word inside the angle brackets would
1125+
// be read as literal text. drop the characters that can not appear in a header
1126+
// at all, but leave HT alone, it separates the ids of a multi id value
1127+
value = (value || '')
1128+
.toString()
1129+
.replace(/\r?\n|\r/g, ' ')
1130+
.replace(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/g, '');
11251131

11261132
if (value.charAt(0) !== '<') {
11271133
value = '<' + value;
@@ -1141,6 +1147,7 @@ class MimeNode {
11411147
elm = (elm || '')
11421148
.toString()
11431149
.replace(/\r?\n|\r/g, ' ')
1150+
.replace(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/g, '')
11441151
.trim();
11451152
return elm.replace(/<[^>]*>/g, str => str.replace(/\s/g, '')).split(/\s+/);
11461153
})
@@ -1163,7 +1170,7 @@ class MimeNode {
11631170
}
11641171

11651172
value = (value || '').toString().replace(/\r?\n|\r/g, ' ');
1166-
return this._encodeWords(value);
1173+
return this._encodeHeaderText(value);
11671174

11681175
case 'Content-Type':
11691176
case 'Content-Disposition':
@@ -1172,8 +1179,7 @@ class MimeNode {
11721179

11731180
default:
11741181
value = (value || '').toString().replace(/\r?\n|\r/g, ' ');
1175-
// encodeWords only encodes if needed, otherwise the original string is returned
1176-
return this._encodeWords(value);
1182+
return this._encodeHeaderText(value);
11771183
}
11781184
}
11791185

@@ -1278,6 +1284,21 @@ class MimeNode {
12781284
return name;
12791285
}
12801286

1287+
/**
1288+
* Encodes an unstructured header value. Such a value can only carry VCHAR and WSP, so a
1289+
* control char or DEL has to be forced into the mime encoded word that a non-ascii value
1290+
* would get anyway. HT stays as it is, it is valid folding whitespace here.
1291+
*
1292+
* @param {String} value Header value to encode
1293+
* @returns {String} Mime word encoded string if needed
1294+
*/
1295+
_encodeHeaderText(value) {
1296+
return /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/.test(value)
1297+
? mimeFuncs.encodeWord(value, this._getTextEncoding(value), 52)
1298+
: // encodeWords only encodes if needed, otherwise the original string is returned
1299+
this._encodeWords(value);
1300+
}
1301+
12811302
/**
12821303
* If needed, mime encodes the name part
12831304
*

test/mime-node/mime-node-test.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,26 @@ describe('MimeNode Tests', { timeout: 50 * 1000 }, () => {
657657
});
658658
});
659659

660+
it('should not leave a control char anywhere in the headers', (t, done) => {
661+
// a header carries VCHAR and WSP only. every field below used to pass these through
662+
let mb = new MimeNode('text/plain')
663+
.setHeader({
664+
subject: 'in\x01voice',
665+
'x-custom': 'a\x7fb',
666+
'message-id': '<a\x01b@example.test>',
667+
'in-reply-to': '<c\x1bd@example.test>',
668+
references: '<e\x00f@example.test>'
669+
})
670+
.setContent('x');
671+
672+
mb.build((err, msg) => {
673+
assert.ok(!err);
674+
const headers = msg.toString().split('\r\n\r\n')[0];
675+
assert.strictEqual(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/.test(headers), false);
676+
done();
677+
});
678+
});
679+
660680
it('should encode filename with a tab', (t, done) => {
661681
let mb = new MimeNode('application/pdf', {
662682
filename: 'in\tvoice.pdf'
@@ -1376,17 +1396,50 @@ describe('MimeNode Tests', { timeout: 50 * 1000 }, () => {
13761396
assert.strictEqual(mb._encodeHeaderValue('x-my', 'test jõgeva value'), '=?UTF-8?Q?test_j=C3=B5geva_value?=');
13771397
});
13781398

1399+
it('should encode a control char', () => {
1400+
// a header value can only carry VCHAR and WSP, so these have to become an encoded word
1401+
let mb = new MimeNode();
1402+
assert.strictEqual(mb._encodeHeaderValue('x-my', 'in\x01voice'), '=?UTF-8?Q?in=01voice?=');
1403+
assert.strictEqual(mb._encodeHeaderValue('x-my', 'a\x7fb'), '=?UTF-8?Q?a=7Fb?=');
1404+
});
1405+
1406+
it('should keep a tab in a header value', () => {
1407+
// HT is valid folding whitespace here, unlike in a header parameter
1408+
let mb = new MimeNode();
1409+
assert.strictEqual(mb._encodeHeaderValue('x-my', 'a\tb'), 'a\tb');
1410+
});
1411+
13791412
it('should format references', () => {
13801413
let mb = new MimeNode();
13811414
assert.strictEqual(mb._encodeHeaderValue('references', 'abc def'), '<abc> <def>');
13821415
assert.strictEqual(mb._encodeHeaderValue('references', ['abc', 'def']), '<abc> <def>');
13831416
});
13841417

1418+
it('should strip control chars from references', () => {
1419+
// a msg-id is structured, an encoded word inside the brackets would be literal text
1420+
let mb = new MimeNode();
1421+
assert.strictEqual(mb._encodeHeaderValue('references', 'a\x00b c\x7fd'), '<ab> <cd>');
1422+
});
1423+
1424+
it('should keep tab separated references apart', () => {
1425+
// HT separates the ids of an unfolded References header, so stripping it would
1426+
// merge them into a single unusable token
1427+
let mb = new MimeNode();
1428+
assert.strictEqual(mb._encodeHeaderValue('references', '<a@x.test>\t<b@x.test>'), '<a@x.test> <b@x.test>');
1429+
assert.strictEqual(mb._encodeHeaderValue('in-reply-to', '<a@x.test>\t<b@x.test>'), '<a@x.test>\t<b@x.test>');
1430+
});
1431+
13851432
it('should format message-id', () => {
13861433
let mb = new MimeNode();
13871434
assert.strictEqual(mb._encodeHeaderValue('message-id', 'abc'), '<abc>');
13881435
});
13891436

1437+
it('should strip control chars from message-id', () => {
1438+
let mb = new MimeNode();
1439+
assert.strictEqual(mb._encodeHeaderValue('message-id', 'a\x01b'), '<ab>');
1440+
assert.strictEqual(mb._encodeHeaderValue('in-reply-to', 'c\x1bd'), '<cd>');
1441+
});
1442+
13901443
it('should format addresses', () => {
13911444
let mb = new MimeNode();
13921445
assert.strictEqual(

test/nodemailer/list-headers-test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,21 @@ describe('List-* header comment CRLF injection', () => {
172172
assert.strictEqual(await listIdValue('a\x7fb'), 'List-ID: =?UTF-8?Q?a=7Fb?= <mylist.example.test>');
173173
assert.strictEqual(await listUrlValue('a\x7fb'), 'List-Unsubscribe: <https://example.test/u> (=?UTF-8?Q?a=7Fb?=)');
174174
});
175+
176+
it('should strip control chars from a List-* url', async () => {
177+
// the angle brackets around a url are not a quoting construct and a url has no
178+
// way to carry these, so anything left here would land in the header raw
179+
const raw = await send({
180+
from: 'sender@example.test',
181+
to: 'recipient@example.test',
182+
subject: 'list unsubscribe',
183+
list: { unsubscribe: { url: 'https://example.test/u\x01n\x7fsub' } },
184+
text: 'body'
185+
});
186+
187+
const line = raw.split('\r\n').find(l => /^List-Unsubscribe:/i.test(l));
188+
assert.strictEqual(line, 'List-Unsubscribe: <https://example.test/unsub>');
189+
});
175190
});
176191

177192
it('should keep a benign comment intact in the List-* header', async () => {

0 commit comments

Comments
 (0)