Nodemailer: RFC 5322 comment inside an angle-bracket address is never stripped

Summary

Nodemailer's address parser has three published, patched fixes for RFC 5322 comment ( ... ) handling inside an addr-spec: GHSA-cc9r-2j5m-2m83 (comment glued two domain atoms into one), GHSA-g57g-f23g-4646 (the same glue for a quoted local part), and the related ReDoS fixes GHSA-prgh-xp8r-p3m5 / GHSA-v53p-9fqp-m79j. All three fixes live in _handleAddress()'s joins / _recoverAddrSpec() logic in src/addressparser/index.ts, and that logic is exercised whenever the local text-accumulation path in the tokenizer runs.

That logic is never reached at all for the single most common way a recipient is written: a display name followed by an angle-bracket address, Name <local@domain>. When the input inside the angle brackets itself contains an RFC 5322 comment, e.g.

Legit User <user@good-corp.com(x)evil.com>

the comment is not recognized as an operator, is not stripped, and the parenthesis characters survive verbatim inside the parsed address. That value is used as-is for envelope.to and is written raw onto the wire in the RCPT TO SMTP command. I confirmed this end to end:

• against nodemailer's own jsonTransport/streamTransport (envelope.to and the emitted To: header both carry the malformed value), and
• against a real SMTP server (aiosmtpd), where the malformed value is fatal to the transaction (501 Syntax: RCPT TO: <address> [SP <mail-parameters>], all recipients rejected).

This reproduces unmodified on every version I tested from 6.9.16 through the current 10.0.12 HEAD, i.e. it predates and survives all three of the address-parser advisories above unchanged, because it is a different root cause in a code path those fixes never touch.

Root cause

1. The tokenizer has a single-slot operator stack

src/addressparser/index.ts, Tokenizer.checkChar():

} else if (!this.operatorExpecting && !this.inDomainLiteral && chr in this.operators) {
    this.node = { type: 'operator', value: chr };
    this.list.push(this.node);
    this.node = null;
    this.operatorExpecting = this.operators[chr];
    this.escaped = false;
    return;
}

(currently around line 670)

operatorExpecting is a single string, not a stack. Once < opens the address scope, operatorExpecting is set to '>'. The branch above that recognizes a new operator character (such as () is gated on !this.operatorExpecting, which is now false. So while inside <...>, a ( is never tokenized as an operator at all — it is just an ordinary character appended to the running text token, together with everything up to the next >. The RFC 5322 comment is not recognized, not entered, and not stripped: it rides straight through as literal text.

2. The _handleAddress state machine loses the address state on any operator boundary

src/addressparser/index.ts, inside _handleAddress() (currently around line 365-388):

switch (token.value) {
    case '<':
        state = 'address';
        insideQuotes = false;
        break;
    case '(':
        state = 'comment';
        insideQuotes = false;
        break;
    ...
    default:
        state = 'text';
        insideQuotes = false;
        break;
}

Even where a comment is recognized (outside angle brackets), closing it with ) falls through to default and resets state to 'text', not back to whatever state was active before the comment opened. This is why the fixed cases (bare address, quoted local part) only work at all: the leaked comment tail lands in the free-text bucket, where _recoverAddrSpec()/the ADDR_SPEC extraction logic can find and strip it. Inside <...>, none of that applies in the first place (point 1), so there is no text-bucket safety net to fall back on — the whole point 1/point 2 interaction is what makes the angle-bracket case categorically different from, and unprotected by, the three existing fixes.

3. The one existing safety net requires whitespace, and this value has none

_recoverAddrSpec() (currently around line 283) is the function all three prior fixes rely on to peel a clean addr-spec back out of a polluted value. Its very first line is:

function _recoverAddrSpec(data: { address: string; text: string }): void {
    if (!HAS_WHITESPACE.test(data.address)) {
        return;
    }
    ...

user@good-corp.com(x)evil.com contains no whitespace at all — the comment content x had none, and nothing in the angle-bracket path inserts any — so _recoverAddrSpec bails out immediately and never even looks at the value. There is no cleanup path for this shape.

Proof of Concept

Environment: Node.js, current master (commit 30e4cfd5902fd5da4d1bc76dc8e4bc92d288adef, package.json version 10.0.12), no special transport configuration required.

Parser-level confirmation

const addressparser = require('nodemailer/lib/addressparser');
console.log(addressparser('Legit User <user@good-corp.com(x)evil.com>'));
// [ { address: 'user@good-corp.com(x)evil.com', name: 'Legit User' } ]

Compare with the already-fixed, unbracketed form of the exact same bytes:

console.log(addressparser('user@good-corp.com(x)evil.com'));
// [ { address: 'user@good-corp.com', name: 'evil.com' } ]   <- correctly stripped

sendMail / envelope confirmation (jsonTransport, no network)

const nodemailer = require('nodemailer');

(async () => {
    const t = nodemailer.createTransport({ jsonTransport: true });
    const info = await t.sendMail({
        from: 'app@good-corp.com',
        to: 'Legit User <user@good-corp.com(x)evil.com>',
        subject: 'hi',
        text: 'x'
    });
    console.log('envelope.to:', info.envelope.to);
})();

Observed output:

envelope.to: [ 'user@good-corp.com(x)evil.com' ]

The raw streamTransport message shows the same value in the emitted header:

From: app@good-corp.com
To: Legit User <user@good-corp.com(x)evil.com>
Subject: hi

Wire-level confirmation against a real SMTP server

const net = require('net');
const nodemailer = require('nodemailer');

const server = net.createServer(sock => {
    let buf = '';
    sock.write('220 sink ESMTP\r\n');
    sock.on('data', d => {
        buf += d;
        let i;
        while ((i = buf.indexOf('\r\n')) >= 0) {
            const line = buf.slice(0, i);
            buf = buf.slice(i + 2);
            const u = line.toUpperCase();
            if (u.startsWith('EHLO')) sock.write('250-sink\r\n250 8BITMIME\r\n');
            else if (u.startsWith('MAIL')) sock.write('250 ok\r\n');
            else if (u.startsWith('RCPT')) { console.log('wire RCPT TO :', line); sock.write('250 ok\r\n'); }
            else if (u.startsWith('DATA')) sock.write('354 go\r\n');
            else if (line === '.') sock.write('250 ok\r\n');
            else if (u.startsWith('QUIT')) { sock.write('221 bye\r\n'); sock.end(); }
            else sock.write('250 ok\r\n');
        }
    });
});

server.listen(0, '127.0.0.1', async () => {
    const t = nodemailer.createTransport({ host: '127.0.0.1', port: server.address().port, secure: false });
    const info = await t.sendMail({ from: 'app@good-corp.com', to: 'Legit User <user@good-corp.com(x)evil.com>', subject: 'hi', text: 'x' });
    console.log('envelope.to:', info.envelope.to);
    t.close(); server.close();
});

Observed:

wire RCPT TO : RCPT TO:<user@good-corp.com(x)evil.com>
envelope.to: [ 'user@good-corp.com(x)evil.com' ]

The same recipient string sent at a real, conformant SMTP server (aiosmtpd, a widely used reference-grade Python SMTP implementation) instead of the sink above:

Error: Can't send mail - all recipients were rejected: 501 Syntax: RCPT TO: <address> [SP <mail-parameters>]
code: 'EENVELOPE', responseCode: 501, command: 'RCPT TO',
rejected: [ 'user@good-corp.com(x)evil.com' ]

So against a strict receiver the immediate, concretely demonstrated outcome is that the message is bounced: an ordinary-looking recipient address that a sender/application believes is valid (and, see below, that some app-side validators do read as valid, at good-corp.com) silently fails to send.

Interpretation-conflict confirmation (same class as GHSA-cc9r-2j5m-2m83)

The same string, read by two other parsers an application might use to gate on the recipient's domain, produces two more, mutually different readings:

>>> import email.utils
>>> email.utils.parseaddr('Legit User <user@good-corp.com(x)evil.com>')
('Legit User (x)', 'user@good-corp.comevil.com')

>>> import email.policy, email.parser
>>> msg = email.parser.Parser(policy=email.policy.default).parsestr(
...     'To: Legit User <user@good-corp.com(x)evil.com>\n\n')
>>> addr = msg['To'].addresses[0]
>>> addr.username, addr.domain
('user', 'good-corp.com')

So for the exact same bytes:

This is the exact "which parser sees what" comparison table from GHSA-cc9r-2j5m-2m83, reproduced through a different, unpatched code path: an application validating with email.policy.default without checking .defects (a documented pattern in that advisory) approves good-corp.com and hands the string to Nodemailer, which then emits a different, malformed value on the wire.

Version range

The built addressparser/sendMail output was tested for this exact input across published releases:

Unchanged across every one of the three address-parser advisory fixes, because none of them touch the tokenizer's single-slot operatorExpecting state or the _handleAddress state-machine reset on ).

Impact

• Confirmed, demonstrated impact: a recipient address that contains a perfectly legal RFC 5322 comment inside angle brackets — the single most common way a To/Cc/Bcc/From/Reply-To value is written (Display Name <user@domain>) — is parsed into a value containing SMTP-illegal characters ((, )) that a conformant receiving SMTP server rejects outright, silently failing message delivery to what the sender/application believed was a valid, already-validated recipient (CWE-20, availability/reliability impact on outbound mail).
• Demonstrated interpretation conflict (CWE-436): the malformed value that reaches envelope.to and the wire RCPT TO command disagrees with how other parsers, including ones explicitly called out as an at-risk validation pattern in GHSA-cc9r-2j5m-2m83 (Python's email.policy.default without defect inspection), read the exact same bytes. An application gating outbound mail on "is the recipient's domain in my allow-list" using such a validator can approve a value that Nodemailer then sends corrupted, and depending on the receiving MTA's own comment handling, a different domain than either the sender or the validator believed was being used.
• Reachability: every code path that ends up calling MimeNode._parseAddresses() — to, cc, bcc, from, replyTo, and the explicit envelope option — is affected identically, since they all funnel through the same addressparser()/_handleAddress() logic.

Suggested fix

The underlying issue is architectural: Tokenizer.operatorExpecting is a single value, so it cannot represent "currently inside an angle-address, and also currently inside a comment nested within it." A minimal fix is to make the tokenizer maintain a stack of expected closing characters (pushing ')' when entering a comment while another operator scope, e.g. '>', is already open, and popping back to the outer expectation on )), so a comment can be recognized and stripped no matter what scope it is nested inside. _handleAddress's state machine would then need to restore the previous state on a comment close instead of unconditionally resetting to 'text', so recognized-but-stripped comment content inside <...> continues to accumulate as address rather than leaking into text.

As defense in depth, _recoverAddrSpec() (or an equivalent check run specifically on data.address for the angle-bracket case) should also reject/clean any parsed address value that still contains RFC 5322 special characters such as (/) regardless of whether whitespace is present, since envelope.to is used directly to build an SMTP command and any character outside Mailbox (RFC 5321 §4.1.2) reaching that string is already the wrong outcome, independent of how it got there.

Regression test suggestion

Name <user@example.com(x)evil.com> and the bcc/cc/from/replyTo/envelope equivalents should all parse to { address: 'user@example.com', name: 'Name evil.com' } or be rejected outright by sendMail, never silently carry (/) into envelope.to.