Potential fix for code scanning alert no. 27: Prototype-polluting assignment#28
Potential fix for code scanning alert no. 27: Prototype-polluting assignment#28
Conversation
…ignment Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
📦 Bundle Size ReportTotal dist size: 1.3M View detailed breakdown |
|
📊 Code Coverage ReportCoverage: 99.44% ✅ Great coverage! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| current[keys.at(-1)!] = value; | ||
| current[lastKey] = value; |
Check warning
Code scanning / CodeQL
Prototype-polluting function Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
General approach
To harden against prototype pollution, the deep assignment function must:
- Reject dangerous property names in any segment of the path (already done).
- Avoid traversing or depending on inherited properties when creating intermediate objects.
- Ensure intermediate containers are safe (no implicit link to
Object.prototype).
Best concrete fix in this file
Within setNestedValue:
- Replace
if (!(key in current))with a check that only considers own properties, e.g.Object.prototype.hasOwnProperty.call(current, key). This prevents the code from following or relying on inherited properties that might be polluted. - Keep the existing blocking of
__proto__,constructor, andprototypeand the use ofObject.create(null)for new intermediate objects. - We do not need new imports;
Object.prototype.hasOwnPropertyis globally available. - The external behavior remains the same for normal objects: if an intermediate key is absent as an own property, we still create an object; if it exists as an own property, we reuse it. The only change is that inherited properties are now treated as absent, which is safer and consistent with typical expectations for nested data structures.
Specific change
In src/utils/helpers/DiffUtils.ts, in setNestedValue, adjust the loop over keys so that:
if (!(key in current)) {
current[key] = Object.create(null);
}becomes:
if (!Object.prototype.hasOwnProperty.call(current, key)) {
current[key] = Object.create(null);
}No other changes are needed.
| @@ -218,7 +218,7 @@ | ||
|
|
||
| for (let i = 0; i < keys.length - 1; i++) { | ||
| const key = keys[i]; | ||
| if (!(key in current)) { | ||
| if (!Object.prototype.hasOwnProperty.call(current, key)) { | ||
| current[key] = Object.create(null); | ||
| } | ||
| current = current[key] as Record<string, unknown>; |
| } | ||
|
|
||
| current[keys.at(-1)!] = value; | ||
| current[lastKey] = value; |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
Copilot Autofix
AI 5 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
📊 Code Coverage ReportCoverage: % ❌ Low coverage - please add more tests |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
3 participants
Potential fix for https://github.com/sebamar88/bytekit/security/code-scanning/27
General fix: Ensure that no untrusted path segment is ever used as a property name that could affect
Object.prototype. That means validating every component of the dot‑separated path (including the last one) against a deny‑list of prototype‑polluting keys and rejecting/ignoring invalid paths before performing any assignment.Best concrete fix here:
BLOCKED_KEYSdeny‑list.keys.at(-1)!with an explicitlastKeyvariable.lastKeyonly after it has been verified as safe.This preserves existing semantics (it still silently skips unsafe paths and uses
Object.create(null)for new nested objects), but makes the sink at line 223 obviously unreachable for any prototype‑polluting key, and clearer for static analysis.Concretely in
src/utils/helpers/DiffUtils.ts:setNestedValue:keys, derivelastKey = keys[keys.length - 1].!lastKey.keys(includinglastKey) againstBLOCKED_KEYS.lastKeyinstead ofkeys.at(-1)!for the final assignment.No new imports or helper methods are required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.