Fix proxy resolution, escodegen crash, and cache collision bugs#3
Open
erikbilyk wants to merge 11 commits intoctrl-escp:mainfrom
Open
Fix proxy resolution, escodegen crash, and cache collision bugs#3erikbilyk wants to merge 11 commits intoctrl-escp:mainfrom
erikbilyk wants to merge 11 commits intoctrl-escp:mainfrom
Conversation
…nt cross-scope collisions
…replacement nodes
…ough another reference
…r Null and RegExp nodes
ds.js: Proxy resolution now correctly preserves proxy variables when the target's references are modified (commit 3c079ae), producing slightly different output where ~25 incorrect proxy replacements no longer happen. newFunc.js: Function constructor bodies containing return statements now parse successfully (commit cdc4082), enabling additional transformations that were previously skipped.
Add 14 regression tests covering each patched module: - resolveProxyVariables: target refs modified (TN-6), shadowing (TN-7) - resolveProxyReferences: target refs modified (TN-10), shadowing (TN-11) - replaceIdentifierWithFixedValue: conditional reads (TP-7), Identifier assigns (TP-8) - replaceCallExpressionsWithUnwrappedIdentifier: used params (TN-5) - replaceNewFuncCallsWithLiteralContent: return body (TP-6) - resolveLocalCalls: AssignmentExpression callee (TP-8) - resolveMemberExpressionsLocalReferences: property write (TN-7) - createNewNode: invalid RegExp validation - Sandbox: unique ID (TP-11) - evalInVm: sandbox cache isolation (TP-15) - getDeclarationWithContext: nodeId-only cache (TP-9) Fix samples.test.js Windows path handling using dirname(fileURLToPath()).
Owner
|
@erikbilyk thank you for this submission. If you can, please break this PR to multiple PRs, each for a specific issue or group of related issues, and their accompanying tests. I find it a bit hard to review all of the changes grouped together. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses multiple issues found when deobfuscating large real-world scripts:
Cross-sandbox evalInVm cache collisions — Cache key now includes sandbox ID to prevent cached results from being shared across different sandbox contexts
Cross-scope getDeclarationWithContext cache collisions — Removed src-based cache fallback that caused nodes sharing identical source code but in different scopes to improperly share context
Proxy resolution correctness — Implements target variable shadowing checks, target reference modification checks, self-replacement skip logic, and batch validation
escodegen crash on Null literals — Added missing
value: nullproperty to Null case in createNewNode, plus recursive validation of replacement node treesFunction constructor body parsing — Implements function-body wrapper fallback enabling patterns like
new Function("return this")()to parse correctlyreplaceIdentifierWithFixedValue — Now supports Identifier assignments alongside Literals, checking conditional context only for write references
unwrapFunctionShells parameter check — Skip unwrapping call expressions when function parameters are used in the function body, preventing incorrect inlining
resolveMemberExpressions property assignment check — Skip resolving member expressions whose property is assigned through another reference, preventing premature resolution
Regression tests — Added 14 regression tests covering all patches, with cross-platform path handling
Tested against multiple large obfuscated JavaScript files, confirming proxy resolution correctness, elimination of escodegen crashes, and proper Function constructor unwrapping.