fix(security): 2 improvements across 2 files#1681
Conversation
- Security: Potential Prototype Pollution in objectDestroy - Security: Unsafe Regular Expression Construction in NumberLiteralHelper Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Potential Prototype Pollution in objectDestroy - Security: Unsafe Regular Expression Construction in NumberLiteralHelper Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Deploy Preview for hyperformula-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fd8eb55. Configure here.
| for (const key of Object.keys(object)) { | ||
| if (!Object.prototype.hasOwnProperty.call(object, key)) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Redundant hasOwnProperty check after Object.keys
Low Severity
The hasOwnProperty guard is redundant here because Object.keys() already returns only an object's own enumerable properties by specification. The condition !Object.prototype.hasOwnProperty.call(object, key) can never be true for keys returned by Object.keys(object), making this a dead branch that adds unnecessary complexity without any protective benefit.
Reviewed by Cursor Bugbot for commit fd8eb55. Configure here.
|
Hi @tomaioo Thank you very much for taking the time to propose a solution. Could you please also sign our CLA? Without the signature, we won't be able to merge the code. |
2 participants
Summary
fix(security): 2 improvements across 2 files
Problem
Severity:
Medium| File:src/Destroy.ts:L6The
objectDestroyfunction iterates over all object entries and deletes properties or replaces methods. If this function is ever called with an object that has a prototype chain, or ifObject.entriesincludes inherited properties (though it doesn't by default), this could lead to unexpected behavior. More critically, the function acceptsanytype and doesn't validate the input, which could cause issues if called with null/undefined or non-object types.Solution
Add input validation to ensure
objectis a non-null object. Consider usingObject.keysinstead ofObject.entriesfor better control, and explicitly checkhasOwnPropertyto avoid touching prototype properties.Changes
src/Destroy.ts(modified)src/NumberLiteralHelper.ts(modified)Note
Low Risk
Small defensive changes to destroy teardown and number-pattern construction; no auth, data, or API contract changes.
Overview
Hardens instance teardown in
objectDestroy(used byHyperFormula.destroy()): non-objects are ignored, iteration uses own keys only, and methods are stubbed without touching inherited properties.Number parsing now escapes all regex metacharacters in configured thousand/decimal separators when building number patterns, instead of only treating
.specially—so unusual separator characters cannot break or abuse the constructedRegExp.Reviewed by Cursor Bugbot for commit fd8eb55. Bugbot is set up for automated code reviews on this repo. Configure here.