🛡️ Sentinel: [CRITICAL] Fix prototype pollution in Lua labels#41
🛡️ Sentinel: [CRITICAL] Fix prototype pollution in Lua labels#41ericbfriday wants to merge 1 commit into
Conversation
Prevent prototype pollution by initializing the Lua labels dictionary using `Object.create(null)` instead of a standard object literal `{}`. This ensures that user-provided string keys, such as `__proto__`, cannot modify the prototype chain, avoiding potential bypasses or unintended behavior. Provides a fallback to `{}` in older environments. Also includes a Sentinel journal entry documenting the learning.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR mitigates a prototype-pollution risk in luaparse’s control-flow label tracking by ensuring the per-scope labels dictionary is not backed by Object.prototype, preventing malicious Lua label names (e.g., __proto__) from mutating prototypes during parsing.
Changes:
- Initialize
FullFlowContextscopelabelsas a null-prototype dictionary to prevent prototype pollution via user-controlled label names. - Add a Sentinel note documenting the vulnerability and the intended prevention approach.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
luaparse.js |
Switches label-scope dictionary initialization to a null-prototype object to harden label/goto tracking against prototype pollution. |
.jules/sentinel.md |
Documents the discovered issue and recommended prevention technique for future reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FullFlowContext.prototype.pushScope = function (isLoop) { | ||
| var scope = { | ||
| labels: {}, | ||
| labels: Object.create ? Object.create(null) : {}, |
| FullFlowContext.prototype.pushScope = function (isLoop) { | ||
| var scope = { | ||
| labels: {}, | ||
| labels: Object.create ? Object.create(null) : {}, |
| ## 2024-05-31 - [Prevent Prototype Pollution in Lua Labels] | ||
| **Vulnerability:** The Lua labels dictionary was initialized using an object literal (`{}`), which could allow an attacker to bypass checks or pollute prototypes if user-supplied string keys (such as `"__proto__"`) are used. | ||
| **Learning:** Using `{}` for dictionaries with user-controlled keys exposes the object to prototype modification through the `__proto__` property, even if `hasOwnProperty` is used for existence checks. | ||
| **Prevention:** Always initialize dictionaries meant to store user-supplied string keys with `Object.create ? Object.create(null) : {}` to prevent prototype pollution and fallback gracefully. |
|
Closing as duplicate. The prototype pollution fix has been merged in PR #46. |
🚨 Severity: CRITICAL
💡 Vulnerability: The Lua labels dictionary in
luaparse.jswas initialized using a standard object literal ({}). This could allow an attacker to pollute the object's prototype by providing user-supplied string keys like"__proto__", potentially bypassing property checks or causing unintended side effects within the parser's scoping logic.🎯 Impact: If exploited via a crafted Lua script containing specific label names (e.g.,
::__proto__::), this could lead to logic errors in the parser, evasion of label uniqueness checks, or prototype pollution on the dictionary itself.🔧 Fix: The initialization in
FullFlowContext.prototype.pushScopewas updated to securely useObject.create ? Object.create(null) : {}. This approach creates a dictionary without a prototype chain, rendering keys like"__proto__"harmless while maintaining compatibility in older JavaScript runtimes.✅ Verification: Verified by executing the full test suite (
npm run test) and benchmarking (npm run bench:luast) after installing dependencies and building workspaces, confirming that all 2447 assertions continue to pass and performance remains unaffected. Temporary debug scripts were successfully cleaned up.PR created automatically by Jules for task 11891178881477035320 started by @ericbfriday