DoS via unvalidated toString() calls may still exist in some code paths (related to CVE-2024-21521)

[decsecre583]

Issue description

Hi,

We are a research team investigating plausible but incomplete security patches. We have identified a potential issue with the DoS fix for CVE-2024-21521 and have manually confirmed the pattern only through our analysis. Would you kindly help confirm the issue?

Summary

The DoS fix for CVE-2024-21521 adds argument count and type checks at entry points, but internal functions might still call toString() on user input without validation. This could allow DoS attacks via crafted objects with malicious toString() implementations.

Vulnerability Pattern

// VULNERABLE: toString() called on arbitrary object
function process(input) {
    const data = input.toString();  // Calls user-controlled toString!
    // ...
}

// Attacker provides:
{
    toString: function() {
        while(true) {}  // Infinite loop = DoS
    }
}

The Fix Pattern

if (typeof input !== 'string' && !Buffer.isBuffer(input)) {
    throw new TypeError('Input must be string or Buffer');
}

Proof of Concept

/*
 * DoS pattern verification for discordjs/opus
 * Run: node test.js
 */

class MaliciousObject {
    constructor(behavior) {
        this.behavior = behavior;
    }

    toString() {
        if (this.behavior === 'slow') {
            let count = 0;
            while (count < 1000000) count++;
            return 'looped';
        } else if (this.behavior === 'huge') {
            return 'A'.repeat(10 ** 7);
        } else if (this.behavior === 'exception') {
            throw new Error('DoS via exception');
        }
        return 'normal';
    }
}

// VULNERABLE pattern
function processVulnerable(input) {
    return String(input);  // Calls toString()
}

// FIXED pattern
function processFixed(input) {
    if (typeof input !== 'string' && !Buffer.isBuffer(input)) {
        throw new TypeError('Input must be string or Buffer');
    }
    return input;
}

// Tests
console.log('Testing vulnerable pattern:');
try {
    processVulnerable(new MaliciousObject('slow'));
    console.log('  Processed malicious object (DoS possible)');
} catch (e) {
    console.log('  Error:', e.message);
}

console.log('\nTesting fixed pattern:');
try {
    processFixed(new MaliciousObject('slow'));
} catch (e) {
    console.log('  Blocked:', e.message);
}

We are concerned

1. Are there internal functions that call toString() or String() on inputs?
2. Are the type checks applied at all entry points?
3. Could prototype pollution bypass the type checks?

Impact

• Attack Vector: Attacker provides object with malicious toString() to any function that coerces input to string
• Exploitation: CPU exhaustion, memory exhaustion, or event loop blocking
• Consequences:
  • Discord bot becomes unresponsive
  • Voice connection drops
  • Service degradation for all users
• Affected Users: Discord bots using @discordjs/opus for voice processing

Suggested Fix

Ensure all code paths that process user input validate types before any string coercion:

function safeProcess(input) {
    if (typeof input !== 'string' && !Buffer.isBuffer(input)) {
        throw new TypeError('Input must be string or Buffer');
    }
    // Now safe to use input
}

We are happy to assist with testing if needed.

Code sample

Versions

= 0.9.0

Issue priority

Medium (should be fixed soon)