Skip to content

Fix proxy resolution, escodegen crash, and cache collision bugs#152

Open
erikbilyk wants to merge 6 commits intoHumanSecurity:mainfrom
erikbilyk:fix/deobfuscation-patches
Open

Fix proxy resolution, escodegen crash, and cache collision bugs#152
erikbilyk wants to merge 6 commits intoHumanSecurity:mainfrom
erikbilyk:fix/deobfuscation-patches

Conversation

@erikbilyk
Copy link

Summary

This PR fixes several bugs encountered when deobfuscating large real-world scripts:

  1. Cross-sandbox evalInVm cache collisions - Cache key now includes sandbox ID to prevent different sandbox contexts from sharing cached results
  2. Cross-scope getDeclarationWithContext cache collisions - Removed src-based cache fallback that caused nodes with identical source but different scopes to share context
  3. Proxy resolution correctness - Added target variable shadowing checks, target reference modification checks, self-replacement skip, and batch validation
  4. escodegen crash on Null literals - Added missing value: null property to Null case in createNewNode, and added recursive validation of replacement node trees
  5. Function constructor body parsing - Added function-body wrapper fallback so new Function("return this")() can be parsed
  6. replaceIdentifierWithFixedValue - Support Identifier assignments alongside Literals, and only check conditional context for write references

Test plan

  • Tested against multiple large obfuscated Roblox JavaScript files
  • Verified proxy resolution produces correct values
  • Verified no more escodegen TypeError crashes
  • Verified Function constructor patterns are correctly unwrapped

@aliceofhartley
Copy link

Just fyi your PR fails 4 tests in the test suite. Also if you're doing work with large javascript files, you might be interested in my forks of restringer, flast, and obfuscation-detector. If you look through the branches I have a set of performance and memory optimizations.

I have PRs open for most of them, so hopefully they'll be merged at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments