From 69224beb0d2dd90bc7a0c13ecdaaab09d0336a2e Mon Sep 17 00:00:00 2001 From: James Feng <47167674+GhostDragon124@users.noreply.github.com> Date: Mon, 8 Jun 2026 10:23:32 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20resolve=204=20dismissed=20CodeQL=20alert?= =?UTF-8?q?s=20=E2=80=94=20URL=20sanitization=20+=20crypto=20bias?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - bingAdapter.ts: fix .hostname.includes('bing.com') → endsWith('.bing.com') to prevent evilbing.com bypass (#367, incomplete-url-substring-sanitization) - Task.ts, LocalMainSessionTask.ts, words.ts: replace modulo bias (bytes[i] % length) with rejection sampling using crypto.randomBytes, eliminating biased-cryptographic-random alerts (#5, #6, #7) Validation: bun run check:fix ✓, bunx tsc --noEmit (0 new errors) ✓, bun run build ✓ --- .../WebSearchTool/adapters/bingAdapter.ts | 2 +- src/Task.ts | 10 ++++-- src/tasks/LocalMainSessionTask.ts | 34 ++++++++++++++----- src/utils/words.ts | 9 +++-- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/packages/builtin-tools/src/tools/WebSearchTool/adapters/bingAdapter.ts b/packages/builtin-tools/src/tools/WebSearchTool/adapters/bingAdapter.ts index 3e90ca643..aaf3673ac 100644 --- a/packages/builtin-tools/src/tools/WebSearchTool/adapters/bingAdapter.ts +++ b/packages/builtin-tools/src/tools/WebSearchTool/adapters/bingAdapter.ts @@ -211,7 +211,7 @@ export function resolveBingUrl(rawUrl: string): string | undefined { // Use proper URL parsing rather than substring matching to prevent bypass try { const parsed = new URL(rawUrl) - if (!parsed.hostname.includes('bing.com')) return rawUrl + if (!(parsed.hostname === 'bing.com' || parsed.hostname.endsWith('.bing.com'))) return rawUrl } catch { // Invalid URL — treat as non-external return undefined diff --git a/src/Task.ts b/src/Task.ts index 196caf365..a71437ba5 100644 --- a/src/Task.ts +++ b/src/Task.ts @@ -97,10 +97,16 @@ const TASK_ID_ALPHABET = '0123456789abcdefghijklmnopqrstuvwxyz' export function generateTaskId(type: TaskType): string { const prefix = getTaskIdPrefix(type) - const bytes = randomBytes(8) + const alphabetLen = TASK_ID_ALPHABET.length + // Use rejection sampling to avoid modulo bias + const maxValid = Math.floor(256 / alphabetLen) * alphabetLen let id = prefix for (let i = 0; i < 8; i++) { - id += TASK_ID_ALPHABET[bytes[i]! % TASK_ID_ALPHABET.length] + let byte: number + do { + byte = randomBytes(1)[0]! + } while (byte >= maxValid) + id += TASK_ID_ALPHABET[byte % alphabetLen] } return id } diff --git a/src/tasks/LocalMainSessionTask.ts b/src/tasks/LocalMainSessionTask.ts index 27398d515..ccecd7172 100644 --- a/src/tasks/LocalMainSessionTask.ts +++ b/src/tasks/LocalMainSessionTask.ts @@ -73,10 +73,16 @@ const DEFAULT_MAIN_SESSION_AGENT: CustomAgentDefinition = { const TASK_ID_ALPHABET = '0123456789abcdefghijklmnopqrstuvwxyz' function generateMainSessionTaskId(): string { - const bytes = randomBytes(8) + const alphabetLen = TASK_ID_ALPHABET.length + // Use rejection sampling to avoid modulo bias + const maxValid = Math.floor(256 / alphabetLen) * alphabetLen let id = 's' for (let i = 0; i < 8; i++) { - id += TASK_ID_ALPHABET[bytes[i]! % TASK_ID_ALPHABET.length] + let byte: number + do { + byte = randomBytes(1)[0]! + } while (byte >= maxValid) + id += TASK_ID_ALPHABET[byte % alphabetLen] } return id } @@ -210,7 +216,10 @@ export function completeMainSessionTask( // Set notified so evictTerminalTask/generateTaskAttachments eviction // guards pass; the backgrounded path sets this inside // enqueueMainSessionNotification's check-and-set. - updateTaskState(taskId, setAppState, task => ({ ...task, notified: true })) + updateTaskState(taskId, setAppState, task => ({ + ...task, + notified: true, + })) emitTaskTerminatedSdk(taskId, success ? 'completed' : 'failed', { toolUseId, summary: 'Background session', @@ -388,10 +397,14 @@ export function startBackgroundSession({ // Aborted mid-stream — completeMainSessionTask won't be reached. // chat:killAgents path already marked notified + emitted; stopTask path did not. let alreadyNotified = false - updateTaskState(taskId, setAppState, task => { - alreadyNotified = task.notified === true - return alreadyNotified ? task : { ...task, notified: true } - }) + updateTaskState( + taskId, + setAppState, + task => { + alreadyNotified = task.notified === true + return alreadyNotified ? task : { ...task, notified: true } + }, + ) if (!alreadyNotified) { emitTaskTerminatedSdk(taskId, 'stopped', { summary: description, @@ -420,7 +433,12 @@ export function startBackgroundSession({ lastRecordedUuid = msg.uuid if (msg.type === 'assistant') { - const contentBlocks = (msg.message?.content ?? []) as Array<{ type: string; text?: string; name?: string; input?: unknown }> + const contentBlocks = (msg.message?.content ?? []) as Array<{ + type: string + text?: string + name?: string + input?: unknown + }> for (const block of contentBlocks) { if (block.type === 'text') { tokenCount += roughTokenCountEstimation(block.text ?? '') diff --git a/src/utils/words.ts b/src/utils/words.ts index aeda8697c..ef5b43aeb 100644 --- a/src/utils/words.ts +++ b/src/utils/words.ts @@ -765,9 +765,12 @@ const VERBS = [ * Generate a cryptographically random integer in the range [0, max) */ function randomInt(max: number): number { - // Use crypto.randomBytes for better randomness than Math.random - const bytes = randomBytes(4) - const value = bytes.readUInt32BE(0) + // Use rejection sampling to avoid modulo bias + const maxValid = Math.floor(0x100000000 / max) * max + let value: number + do { + value = randomBytes(4).readUInt32BE(0) + } while (value >= maxValid) return value % max }