discordjs / discordjs/opus

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

Open
#198 0 comments 0 reactions 0 assignees View on GitHub
bug need repro
Dominant language
C++
Stars
253
Forks
74
PR merge metrics
No merged PRs in 30d

Description

### 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

```javascript
// 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

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

## Proof of Concept

```javascript
/*
* 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:

```javascript
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

```TypeScript

```

### Versions

>= 0.9.0

### Issue priority

Medium (should be fixed soon)

Contributor guide

Open the contributing guide

Research direction

The issue names no repository files or tests; it provides only illustrative test.js functions, processVulnerable and processFixed. Start by locating the actual input-processing entry points in @discordjs/opus, then verify whether each path validates types before coercion and add focused regression coverage for any confirmed path.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.