Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper deobfuscation mapping file format #1491

Closed
NebelNidas opened this issue May 23, 2022 · 7 comments · Fixed by #1505
Closed

Proper deobfuscation mapping file format #1491

NebelNidas opened this issue May 23, 2022 · 7 comments · Fixed by #1505

Comments

@NebelNidas
Copy link
Contributor

As far as I could see, you currently have two options for exporting user-made mappings (renames): the .jadx project file and .jobf mapping files. The issue is, .jadx contains much other information that's not needed/wanted when I just want to get the mappings. And .jobf can neither handle Javadoc/comments nor is it easy to get them generated.
I suggest/request looking into adding .enigma or .tiny mapping format support, which are both currently used by FabricMC in their Minecraft deobfuscation toolchain. Enigma files look like this, for example: https://github.com/FabricMC/yarn/blob/1.19-pre2/mappings/net/minecraft/block/Block.mapping

It should be relatively easy to implement, too, via the mapping-io library. Maybe support for JADX's mappings can be PR'd to there too, so conversion of legacy mapping formats would be possible?

@skylot
Copy link
Owner

skylot commented May 23, 2022

@NebelNidas thanks!
mapping-io looks like a nice library. I will check how I can integrate it into jadx. It will also help me to implement ProGuard mapping input for jadx (#97).

@skylot skylot added this to the TBD milestone May 23, 2022
@NebelNidas
Copy link
Contributor Author

Recaf also has some nice read-only implementations: https://github.com/Col-E/Recaf/tree/master/src/main/java/me/coley/recaf/mapping
Mostly the same like mapping-io, but with more comments (and support for a few other mappings)

@NebelNidas
Copy link
Contributor Author

NebelNidas commented May 28, 2022

I've almost finished the .jadx read support in mapping-io now, but there's one part where I'm stuck: How can I know whether something like this:

"nodeRef": {
  "refType": "METHOD",
  "declClass": "aanu",
  "shortId": "\u003cinit\u003e(Landroid/content/Context;Ljava/lang/String;)V"
},
"codeRef": {
  "attachType": "VAR",
  "index": 131072
},
"newName": "text"

is a method arg or a method var? I mean, the attachType says VAR, but it does so for both method vars and method args for some reason. The other RefTypes are never getting used 🤔

@skylot
Copy link
Owner

skylot commented May 28, 2022

I've almost finished the .jadx read support in mapping-io

Wait, what?
I don't think this is a good idea, because jadx can use internal values for references like instructions offset or register number.
Like in this case for vars, jadx uses a weird combination of register number and SSA variable version, because all variables inside method have these numbers it is a unique identifier. And using this id make no difference between method args and method vars.

P.S. I see that these enums values doesn't use, I just thought they can be useful for comment attach, but still not implement that :(

@NebelNidas
Copy link
Contributor Author

NebelNidas commented May 28, 2022

I don't think this is a good idea, because jadx can use internal values for references like instructions offset or register number

Yeah, but apart from the distinction between method args & vars which relies on index, everything is easily convertible. As far as I could see, if you just read the entries from the .jadx file sequentially, you automatically get the correct method arg order.
So if you could change JADX to correctly output MTH_ARG instead of VAR where applicable, at least the method argument conversion would work :)

@skylot
Copy link
Owner

skylot commented May 28, 2022

@NebelNidas more likely I will just remove MTH_ARG value, because there is no easy way for me to get that info. And as I said, this is not really needed.

@NebelNidas
Copy link
Contributor Author

Hmm, I guess I'll add an Export mappings as... option to JADX directly then

skylot pushed a commit that referenced this issue Jun 11, 2022
…(PR #1505)

* Add option to export mappings as Tiny v2 file

* Comply with JADX's import order conventions

* Only use Java 8 features

* Only use Java 8 features (2)

* Export comments to mappings file

* Method args test (doesn't work)

* Make method arg mapping exports work now

* Use `getTopParentClass()` instead of `getParentClass()`

See #1505 (comment)

* Remove unneeded method load call

* Small code cleanup; initial (broken) support for method vars

* Fixes regarding inner classes

* Add option to export mappings as Enigma directory

* Add option to export mappings as Enigma file/directory

Temporarily move to my mapping-io fork until this PR gets merged: FabricMC/mapping-io#19

* Fix method vars' lv-indices

* Use correct offset value for method var mappings

* Also supply lvt-index for method var mappings

* Clarify why we're using local mapping-io fork; comment out Fabric Maven for now

* Remove unnecessary `public` modifier

* Make an `if` condition less complicated

* Move mapping export code into jadx-gui (for now)

* Make mapping export async; make export menu only clickable when everything is loaded

* Fix export mappings menu field declaration position
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants