fix(dashscope): use URL hostname check instead of regex to avoid ReDoS (CodeQL)#4112
Conversation
CodeQL flagged a polynomial regular-expression alert on the unanchored /([\w-]+\.)?dashscope(-intl)?\.aliyuncs\.com/i pattern introduced in QwenLM#3991 — repeated '-' in the baseUrl could trigger catastrophic backtracking. The unanchored regex was also too permissive: it would incidentally match the dashscope domain appearing anywhere in a URL path, not just in the hostname. Parse the baseUrl with the URL constructor and compare hostname suffixes instead. This eliminates the ReDoS surface entirely and is strictly tighter — only real dashscope[-intl].aliyuncs.com hostnames (and their subdomains) match. Co-authored-by: Qwen-Coder <noreply@alibaba-inc.com>
tanzhenxin
left a comment
There was a problem hiding this comment.
Review
The fix swaps a polynomial-backtracking regex for a parsed-URL hostname check with a dot-bounded suffix match — that's the right shape for this. It removes the CodeQL ReDoS surface, preserves the original case-insensitivity, and meaningfully tightens the predicate: hosts where the dashscope domain only appears in the URL path (e.g. https://evil.example/dashscope.aliyuncs.com/...) and prefix-without-dot-boundary hosts like notdashscope.aliyuncs.com no longer classify as DashScope, so the DashScope-specific headers and session metadata won't be attached to those requests. The proxy shortcut and the surrounding provider plumbing are untouched, and the three regression tests cover the previously-permissive cases.
Verdict
APPROVE — correctly resolves the CodeQL alert and strictly tightens classification in the security-meaningful direction.
QwenLM#4112) [upstream cherry-pick] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes the CodeQL high-severity alert (js/polynomial-redos) that landed on
mainwith #3991.The alert pointed at
packages/core/src/core/openaiContentGenerator/provider/dashscope.ts:44:```ts
const isDashscopeOrigin =
/([\w-]+.)?dashscope(-intl)?.aliyuncs.com/i.test(normalizedBaseUrl);
```
Two problems with this regex against the user-controlled `baseUrl`:
Fix
Parse `baseUrl` with the `URL` constructor and compare the hostname instead. No regex, no ReDoS surface, and strictly tighter:
```ts
let hostname: string | null = null;
try {
hostname = new URL(normalizedBaseUrl).hostname.toLowerCase();
} catch {
hostname = null;
}
const isDashscopeOrigin =
hostname !== null &&
(hostname === 'dashscope.aliyuncs.com' ||
hostname === 'dashscope-intl.aliyuncs.com' ||
hostname.endsWith('.dashscope.aliyuncs.com') ||
hostname.endsWith('.dashscope-intl.aliyuncs.com'));
```
All five existing matching cases (`dashscope.aliyuncs.com`, `dashscope-intl.aliyuncs.com`, `coding.dashscope.aliyuncs.com`, `coding-intl.dashscope-intl.aliyuncs.com`, plus the QWEN_OAUTH / no-baseUrl shortcut) keep passing.
Added three regression tests for the previously-permissive behavior:
Test plan