chore: sync core lib and CLAUDE.md from agent-core#38
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements several Regular Expression Denial of Service (ReDoS) fixes across multiple files by bounding previously unbounded quantifiers and space runs. Additionally, it introduces prototype-pollution guards in lib/enhance/fixer.js to secure the applyAtPath function. Feedback on fixer.js highlights a potential runtime crash vulnerability in applyAtPath if intermediate path segments are missing or not objects, and suggests adding defensive checks to ensure robustness.
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/^((?!__proto__|constructor|prototype)[a-zA-Z_]\w*)\[(\d{1,10})\]$/); | ||
| if (match) { | ||
| if (match && match[1] !== '__proto__' && match[1] !== 'constructor' && match[1] !== 'prototype') { | ||
| current = current[match[1]][parseInt(match[2], 10)]; | ||
| } | ||
| } else { | ||
| if (!isSafeKey(part)) return result; // refuse prototype-polluting traversal | ||
| current = current[part]; | ||
| } | ||
| } | ||
|
|
||
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)[a-zA-Z_]\w*)\[(\d{1,10})\]$/); | ||
| if (match) { | ||
| current[match[1]][parseInt(match[2], 10)] = fixFn(current[match[1]][parseInt(match[2], 10)]); | ||
| // Inline literal guard (not the isSafeKey helper) so CodeQL traces the | ||
| // sanitizing barrier on the assignment below. | ||
| if (match && match[1] !== '__proto__' && match[1] !== 'constructor' && match[1] !== 'prototype') { | ||
| const key = match[1]; | ||
| const idx = parseInt(match[2], 10); | ||
| current[key][idx] = fixFn(current[key][idx]); | ||
| } | ||
| } else { | ||
| } else if (lastPart !== '__proto__' && lastPart !== 'constructor' && lastPart !== 'prototype') { | ||
| current[lastPart] = fixFn(current[lastPart]); | ||
| } |
There was a problem hiding this comment.
The applyAtPath function is vulnerable to runtime crashes (TypeError: Cannot read properties of undefined) if any intermediate path segment does not exist or is not an object. Additionally, if an array access segment fails to match the regex, the loop silently skips updating current, causing the remaining path segments to be resolved against the wrong object, which can lead to incorrect modifications or further crashes.
To ensure robustness and defensive programming, we should validate that current is a non-null object at each step, verify that the key exists before traversing, and return the original result immediately if any segment is invalid or missing.
if (part.includes('[')) {
// Array access
const match = part.match(/^((?!__proto__|constructor|prototype)[a-zA-Z_]\w*)\[(\d{1,10})\]$/);
if (match && match[1] !== '__proto__' && match[1] !== 'constructor' && match[1] !== 'prototype') {
const key = match[1];
const idx = parseInt(match[2], 10);
if (!current || typeof current !== 'object' || !Array.isArray(current[key]) || idx >= current[key].length) {
return result;
}
current = current[key][idx];
} else {
return result;
}
} else {
if (!isSafeKey(part)) return result; // refuse prototype-polluting traversal
if (!current || typeof current !== 'object' || !(part in current)) return result;
current = current[part];
}
}
const lastPart = parts[parts.length - 1];
if (!current || typeof current !== 'object') return result;
if (lastPart.includes('[')) {
const match = lastPart.match(/^((?!__proto__|constructor|prototype)[a-zA-Z_]\w*)\[(\d{1,10})\]$/);
// Inline literal guard (not the isSafeKey helper) so CodeQL traces the
// sanitizing barrier on the assignment below.
if (match && match[1] !== '__proto__' && match[1] !== 'constructor' && match[1] !== 'prototype') {
const key = match[1];
const idx = parseInt(match[2], 10);
if (Array.isArray(current[key]) && idx < current[key].length) {
current[key][idx] = fixFn(current[key][idx]);
}
}
} else if (lastPart !== '__proto__' && lastPart !== 'constructor' && lastPart !== 'prototype') {
if (lastPart in current) {
current[lastPart] = fixFn(current[lastPart]);
}
}
Automated sync of lib/ and CLAUDE.md from agent-core.