Skip to content

fix(dashscope): use URL hostname check instead of regex to avoid ReDoS (CodeQL)#4112

Merged
tanzhenxin merged 1 commit into
QwenLM:mainfrom
qqqys:fix/dashscope-redos-codeql
May 13, 2026
Merged

fix(dashscope): use URL hostname check instead of regex to avoid ReDoS (CodeQL)#4112
tanzhenxin merged 1 commit into
QwenLM:mainfrom
qqqys:fix/dashscope-redos-codeql

Conversation

@qqqys

@qqqys qqqys commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the CodeQL high-severity alert (js/polynomial-redos) that landed on main with #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`:

  1. ReDoS surface. `[\w-]+` is a polynomial-backtracking quantifier; combined with the dotted suffix it can take quadratic time on inputs with many `-` characters. `baseUrl` reaches this check unsanitized (env var, settings, CLI flag).
  2. Too permissive. The pattern is unanchored and the input is the whole URL (scheme + host + path), so an unrelated host with the dashscope domain in its path — e.g. `https://evil.example/dashscope.aliyuncs.com/...\` — also matches and is treated as a trusted DashScope origin, which causes the client to send the DashScope auth/cache headers + session metadata to a third-party server.

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:

  • domain appearing only in URL path → false
  • `notdashscope.aliyuncs.com` (suffix without dot boundary) → false
  • unparseable baseUrl → false

Test plan

  • `vitest run packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts` — 47/47 passing (44 existing + 3 new)
  • `eslint` on changed files — clean
  • `tsc --noEmit` on `packages/core` — clean
  • CodeQL CI on this PR shows the alert resolved

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 tanzhenxin added the type/bug Something isn't working as expected label May 13, 2026

@tanzhenxin tanzhenxin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tanzhenxin tanzhenxin merged commit 15e6222 into QwenLM:main May 13, 2026
8 checks passed
TaimoorSiddiquiOfficial added a commit to TaimoorSiddiquiOfficial/HopCode that referenced this pull request May 13, 2026
QwenLM#4112) [upstream cherry-pick]

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants