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

Kotlin toString renamer produces illegal filenames and may override files during export #2041

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

nitram84
Copy link
Contributor

Steps to reproduce:

  • Open Jadx GUI (latest git), verify that the option [Plugins -> Kotlin Metadata -> rename fields using toString] is enabled
  • Open https://github.com/niranjan94/show-java/releases/download/v3.0.5/ShowJava_v3_0_5.apk (and decompile Jadx itself)
  • Navigate in structure tree to package "kotlin.jvm.internal": All class names in that package are valid
  • Export sources as gradle project -> now there are two empty items in package "kotlin.jvm.internal"
  • Show source code for both items: The class name is " " in both cases and an invalid file "app/src/main/java/kotlin/jvm/internal/ .java" was created.

This PR fixes this issue with a check for valid identifiers before rename. An working alternate fix would be using BetterName, since it includes the valid identifier check, but I wasn't sure if the BetterName metric for java is the same as for kotlin. If you prefer BetterName, please reject my PR and I'll fix this with BetterName.

This PR will fix lots of syntax errors in exported sources.

With other apps I've also seen class names like "." generating a filename "..java", but I haven't found any apps with path traversal attacks yet.

@skylot
Copy link
Owner

skylot commented Nov 10, 2023

@nitram84 thanks, nice fix!
Also, we may need to check this plugin for similar issues and maybe extract same code for node rename into a common method.

About BetterName: it is hard to detect which name is better so that check has limited usage, so in such cases it is better to allow user to select from names list as suggested in #1862 feature.

@skylot skylot merged commit e6d896d into skylot:master Nov 10, 2023
5 checks passed
@nitram84 nitram84 deleted the kotlin_rename_fix branch March 18, 2024 10:25
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