fix: resolve 4 dismissed CodeQL alerts — URL sanitization + crypto bias
- 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 ✓
This commit is contained in:
parent
2249f184ba
commit
69224beb0d
|
|
@ -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
|
||||
|
|
|
|||
10
src/Task.ts
10
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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<LocalMainSessionTaskState>(taskId, setAppState, task => ({ ...task, notified: true }))
|
||||
updateTaskState<LocalMainSessionTaskState>(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<LocalMainSessionTaskState>(taskId, setAppState, task => {
|
||||
alreadyNotified = task.notified === true
|
||||
return alreadyNotified ? task : { ...task, notified: true }
|
||||
})
|
||||
updateTaskState<LocalMainSessionTaskState>(
|
||||
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 ?? '')
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user