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;
    }
    ...

[email protected](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 <[email protected](x)evil.com>'));
// [ { address: '[email protected](x)evil.com', name: 'Legit User' } ]

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

console.log(addressparser('[email protected](x)evil.com'));
// [ { address: '[email protected]', 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: '[email protected]',
        to: 'Legit User <[email protected](x)evil.com>',
        subject: 'hi',
        text: 'x'
    });
    console.log('envelope.to:', info.envelope.to);
})();

Observed output:

envelope.to: [ '[email protected](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.(); }
              (u.()) sock.();
              (line === ) sock.();
              (u.()) { sock.(); sock.(); }
             sock.();
        }
    });
});

server.(, ,  () => {
     t = nodemailer.({ : , : server.()., :  });
     info =  t.({ : , : , : , :  });
    .(, info..);
    t.(); server.();
});

Observed:

wire RCPT TO : RCPT TO:<user@good-corp.com(x)evil.com>
envelope.to: [ '[email protected](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: [ '[email protected](x)evil.com' ]

So against a strict receiver the immediate, concretely demonstrated outcome is that the message is bounced: an ordinary-looking recipient address