T09 · Insecure Skill Coding Practices
Warning
- Location
- src/content-analyzer.js:258
- Finding
- Regular Expression Injection Enables Event-Loop Denial of Service## Vulnerability Details **File Location**: `src/content-analyzer.js`, lines 258-264 **Vulnerability Type**: Regular expression injection and denial of service **Risk Level**: Medium ### Vulnerable Code ```javascript for (const keyword of keywords) { const keywordLower = keyword.toLowerCase(); const occurrences = (contentLower.match(new RegExp(keywordLower, 'g')) || []).length; const density = (occurrences / this._countWords(content)) * 100; ``` ### Technical Analysis The application inserts an attacker-controlled keyword directly into the `RegExp` constructor without escaping regular-expression metacharacters. The keyword is therefore interpreted as executable regular-expression syntax rather than as a literal search term. An invalid expression such as `[` causes the constructor to throw a syntax error. More importantly, a pattern containing nested or ambiguous quantifiers, such as `(a+)+$`, can cause catastrophic backtracking when evaluated against suitably crafted content. Because Node.js evaluates this regular expression synchronously on the main event loop, a computationally expensive match can prevent the process from servicing other requests. Both the content and keyword arrays are exposed through the exported `optimizeContent(content, keywords)` API and the `optimize_content` action handled by `index.js`. No input-size limits, keyword-count limits, regular-expression escaping, or execution timeout protects this operation. ### Attack Path 1. An attacker submits an `optimize_content` request with a crafted keyword such as `(a+)+$`. 2. The attacker supplies content containing a long sequence of `a` characters followed by a nonmatching character, such as `aaaaaaaa...!`. 3. `index.js` forwards the content and keyword array to `SEOEngine.optimizeContent()`. 4. The content analyzer converts the attacker-controlled keyword into a regular expression at `src/content-analyzer.js:263`. 5. Matching the expression ...[truncated 939 chars]
- Remediation
- ## Remediation Suggestions Treat keywords as literal text rather than regular-expression source. Escape all regular-expression metacharacters before constructing a `RegExp`: ```javascript function escapeRegExp(value) { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } for (const keyword of keywords) { if (typeof keyword !== 'string' || keyword.length === 0) { continue; } const escapedKeyword = escapeRegExp(keyword.toLowerCase()); const occurrences = (contentLower.match(new RegExp(escapedKeyword, 'g')) || []).length; const density = (occurrences / Math.max(this._countWords(content), 1)) * 100; } ``` Prefer a literal string-counting implementation using `indexOf()` when regular-expression behavior is unnecessary. This eliminates regular-expression injection entirely. Apply additional defense-in-depth controls: 1. Validate that `content` is a string and `keywords` is an array of strings. 2. Enforce maximum lengths for content and each keyword. 3. Enforce a maximum number of keywords per request. 4. Reject empty or excessively complex input before analysis. 5. Apply request-level rate limits where the package is exposed through an API. 6. Add regression tests using malformed patterns such as `[` and pathological patterns such as `(a+)+$`, verifying that they are processed as literal text. 7. For high-volume services, isolate CPU-intensive analysis in worker threads or separate processes with execution deadlines.