Skip to content

Commit 1150d99

Browse files
committed
fix(addressparser): Fixed addressparser handling of quoted nested email addresses
1 parent 3b8982c commit 1150d99

File tree

2 files changed

+66
-6
lines changed

2 files changed

+66
-6
lines changed

lib/addressparser/index.js

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ function _handleAddress(tokens) {
1515
address: [],
1616
comment: [],
1717
group: [],
18-
text: []
18+
text: [],
19+
textWasQuoted: [] // Track which text tokens came from inside quotes
1920
};
2021
let i;
2122
let len;
23+
let insideQuotes = false; // Track if we're currently inside a quoted string
2224

2325
// Filter out <addresses>, (comments) and regular text
2426
for (i = 0, len = tokens.length; i < len; i++) {
@@ -28,16 +30,25 @@ function _handleAddress(tokens) {
2830
switch (token.value) {
2931
case '<':
3032
state = 'address';
33+
insideQuotes = false;
3134
break;
3235
case '(':
3336
state = 'comment';
37+
insideQuotes = false;
3438
break;
3539
case ':':
3640
state = 'group';
3741
isGroup = true;
42+
insideQuotes = false;
43+
break;
44+
case '"':
45+
// Track quote state for text tokens
46+
insideQuotes = !insideQuotes;
47+
state = 'text';
3848
break;
3949
default:
4050
state = 'text';
51+
insideQuotes = false;
4152
break;
4253
}
4354
} else if (token.value) {
@@ -51,8 +62,14 @@ function _handleAddress(tokens) {
5162
if (prevToken && prevToken.noBreak && data[state].length) {
5263
// join values
5364
data[state][data[state].length - 1] += token.value;
65+
if (state === 'text' && insideQuotes) {
66+
data.textWasQuoted[data.textWasQuoted.length - 1] = true;
67+
}
5468
} else {
5569
data[state].push(token.value);
70+
if (state === 'text') {
71+
data.textWasQuoted.push(insideQuotes);
72+
}
5673
}
5774
}
5875
}
@@ -74,8 +91,12 @@ function _handleAddress(tokens) {
7491
// If no address was found, try to detect one from regular text
7592
if (!data.address.length && data.text.length) {
7693
for (i = data.text.length - 1; i >= 0; i--) {
77-
if (data.text[i].match(/^[^@\s]+@[^@\s]+$/)) {
94+
// Security fix: Do not extract email addresses from quoted strings
95+
// RFC 5321 allows @ inside quoted local-parts like "user@domain"@example.com
96+
// Extracting emails from quoted text leads to misrouting vulnerabilities
97+
if (!data.textWasQuoted[i] && data.text[i].match(/^[^@\s]+@[^@\s]+$/)) {
7898
data.address = data.text.splice(i, 1);
99+
data.textWasQuoted.splice(i, 1);
79100
break;
80101
}
81102
}
@@ -92,10 +113,13 @@ function _handleAddress(tokens) {
92113
// still no address
93114
if (!data.address.length) {
94115
for (i = data.text.length - 1; i >= 0; i--) {
95-
// fixed the regex to parse email address correctly when email address has more than one @
96-
data.text[i] = data.text[i].replace(/\s*\b[^@\s]+@[^\s]+\b\s*/, _regexHandler).trim();
97-
if (data.address.length) {
98-
break;
116+
// Security fix: Do not extract email addresses from quoted strings
117+
if (!data.textWasQuoted[i]) {
118+
// fixed the regex to parse email address correctly when email address has more than one @
119+
data.text[i] = data.text[i].replace(/\s*\b[^@\s]+@[^\s]+\b\s*/, _regexHandler).trim();
120+
if (data.address.length) {
121+
break;
122+
}
99123
}
100124
}
101125
}

test/addressparser/addressparser-test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,4 +309,40 @@ describe('#addressparser', () => {
309309
];
310310
assert.deepStrictEqual(addressparser(input), expected);
311311
});
312+
313+
// Security tests for RFC 5321/5322 quoted local-part handling
314+
it('should not extract email from quoted local-part (security)', () => {
315+
let input = '"[email protected] x"@internal.domain';
316+
let result = addressparser(input);
317+
// Should preserve full address, NOT extract [email protected]
318+
assert.strictEqual(result.length, 1);
319+
assert.strictEqual(result[0].address.includes('@internal.domain'), true);
320+
assert.strictEqual(result[0].address, '[email protected] [email protected]');
321+
});
322+
323+
it('should handle quoted local-part with attacker domain (security)', () => {
324+
let input = '"[email protected]"@legitimate.com';
325+
let result = addressparser(input);
326+
// Should route to legitimate.com, not attacker.com
327+
assert.strictEqual(result.length, 1);
328+
assert.strictEqual(result[0].address.includes('@legitimate.com'), true);
329+
assert.strictEqual(result[0].address, '[email protected]@legitimate.com');
330+
});
331+
332+
it('should handle multiple @ in quoted local-part (security)', () => {
333+
let input = '"a@b@c"@example.com';
334+
let result = addressparser(input);
335+
// Should not extract a@b or b@c
336+
assert.strictEqual(result.length, 1);
337+
assert.strictEqual(result[0].address, 'a@b@[email protected]');
338+
});
339+
340+
it('should handle quoted local-part with angle brackets', () => {
341+
let input = 'Name <"[email protected]"@example.com>';
342+
let result = addressparser(input);
343+
assert.strictEqual(result.length, 1);
344+
assert.strictEqual(result[0].name, 'Name');
345+
// When address is in <>, quotes are preserved as part of the address
346+
assert.strictEqual(result[0].address, '"[email protected]"@example.com');
347+
});
312348
});

0 commit comments

Comments
 (0)