Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const fields = useMemo(() => { | ||
| if (!workflowContext || !dataSource) return []; | ||
|
|
||
| const match = dataSource.match(/\{\{ctx\.([^}]+)\}\}/); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
| if (!current[pathParts[i]]) current[pathParts[i]] = {}; | ||
| current = current[pathParts[i]]; | ||
| } | ||
| current[pathParts[pathParts.length - 1]] = newValue; |
Check warning
Code scanning / CodeQL
Prototype-polluting function Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
In general, to fix unsafe deep assignment / merge functions you must prevent special prototype-related keys (__proto__, constructor, and often prototype) from being used as property names when creating or traversing nested objects. Alternatively, you can restrict recursive merging to properties that already exist as own properties on the destination object. This prevents attackers from reaching and modifying Object.prototype through crafted paths.
For this specific handleSpecChange function in packages/widgets-ui/src/vega/chartBuilder.jsx, the minimal and safest fix is to introduce a small guard that checks each path segment before it is used to access or create properties on current. If any segment is __proto__, constructor, or prototype, we should abort the update and return the previous state unchanged. This preserves existing functionality for all legitimate keys while eliminating the possibility of prototype pollution.
Concretely:
- Inside
handleSpecChange, after computingconst pathParts = updatePath.split('.'), introduce a helperisUnsafeKeyfunction (insidehandleSpecChangeto keep scope local) that returnstruefor__proto__,constructor, orprototype. - When iterating over
pathPartsin theforloop (lines 69–72), before usingpathParts[i]to index intocurrent, checkif (isUnsafeKey(pathParts[i])) return prev;. - Before the final assignment on line 73, check
if (isUnsafeKey(lastKey)) return prev;. - No external dependencies or imports are needed; this is pure JS.
This approach changes only the behavior for unsafe keys and does not affect existing valid usage of handleSpecChange.
| @@ -63,14 +63,28 @@ | ||
| // Deep clone and update | ||
| const nextSpec = JSON.parse(JSON.stringify(prev)); | ||
|
|
||
| // Helper to prevent prototype pollution via special keys | ||
| const isUnsafeKey = (key) => | ||
| key === '__proto__' || key === 'constructor' || key === 'prototype'; | ||
|
|
||
| // Navigate to the target path and apply update | ||
| const pathParts = updatePath.split('.'); | ||
| let current = nextSpec; | ||
| for (let i = 0; i < pathParts.length - 1; i++) { | ||
| if (!current[pathParts[i]]) current[pathParts[i]] = {}; | ||
| current = current[pathParts[i]]; | ||
| const segment = pathParts[i]; | ||
| if (isUnsafeKey(segment)) { | ||
| // Abort update if an unsafe key is encountered | ||
| return prev; | ||
| } | ||
| if (!current[segment]) current[segment] = {}; | ||
| current = current[segment]; | ||
| } | ||
| current[pathParts[pathParts.length - 1]] = newValue; | ||
| const lastKey = pathParts[pathParts.length - 1]; | ||
| if (isUnsafeKey(lastKey)) { | ||
| // Abort update if an unsafe key is used for the final assignment | ||
| return prev; | ||
| } | ||
| current[lastKey] = newValue; | ||
|
|
||
| return nextSpec; | ||
| }); |
3 participants
No description provided.