DoS via unvalidated toString() calls may still exist in some code paths (related to CVE-2024-21521)
- 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
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