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

[gui] Slow check for zip entrynames #2429

Closed
1 of 3 tasks
eybisi opened this issue Mar 6, 2025 · 3 comments
Closed
1 of 3 tasks

[gui] Slow check for zip entrynames #2429

eybisi opened this issue Mar 6, 2025 · 3 comments

Comments

@eybisi
Copy link
Contributor

eybisi commented Mar 6, 2025

Issue details

In Windows, loading apk files with many resource files takes additional time because of the getCanonicalFile function

File canonical = new File(currentPath, entryName).getCanonicalFile();

Following image shows the time that getCanonicalFile took (7sec)
Image
After replacing it with getAbsoluteFile (which doesn't use IO) it took (1sec)
Image

Total load time was 20 sec. 7 seconds makes huge difference.

getCanonicalFile resolves "..", "." and symlinks. In isValidZipEntryName, getCanonicalFile's return value is used to check if entry will be in sub directory of the CWD. Before this ".." is checked. So if we replace it with "getAbsoluteFile" we will be missing only symlinks. If there are no security implications I can create a PR.

Jadx version

dev

Java version

21.0.5

OS

  • Windows
  • Linux
  • macOS
@eybisi eybisi added bug GUI Issues in jadx-gui module labels Mar 6, 2025
@skylot
Copy link
Owner

skylot commented Mar 6, 2025

The current implementation is simple and bulletproof, because it is using system native path resolvers and as a result shows exactly how the system will handle the provided file path. Without such resolving, check will become useless, so I don't want to change this code. But, anyway, if someone else has some arguments and/or other approach feel free to share 🙂

@skylot skylot added enhancement discussion and removed bug GUI Issues in jadx-gui module labels Mar 6, 2025
@eybisi
Copy link
Contributor Author

eybisi commented Mar 6, 2025

While loading resources, does jadx writes that file to filesystem? When saving a project file path is important yes, but what can happen when entry named "../../x" is loaded. Maybe two different checks can be used

@skylot
Copy link
Owner

skylot commented Mar 6, 2025

While loading resources, does jadx writes that file to filesystem?

jadx-cli save all valid resources after loading.
In jadx-gui saving is optional, but showing something that can't be saved may be misleading, so now jadx prepare "final" version of resources so you will get same files after saving.

Now, it is possible to disable zip security checks with environment variable, if you want to view removed resources.

@eybisi eybisi closed this as completed Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants