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

[BUG]When the assets directory contains the Assets folder, the generated apk cannot run #3565

Open
yangbo637829 opened this issue Apr 8, 2024 · 3 comments
Labels

Comments

@yangbo637829
Copy link

Information

  1. Apktool Version (apktool -version) - 2.9.3
  2. Operating System (Mac, Linux, Windows) - mac
  3. APK From? (Playstore, ROM, Other) - playstore
  4. Java Version (java --version) - java 11

Bug

Pay attention to the capitalization of letters about assets and Assets

When the assets directory contains the Assets folder, an additional folder named Assets will be generated at the same level as assets in the generated apk. All non-directory files under the original assets will be placed in the Assets directory.

error code

brut.util.BrutIO#sanitizeFilepath:

final String canonicalEntryPath = new File(directory, entry).getCanonicalPath();

    public static String sanitizeFilepath(final File directory, final String entry) throws IOException, BrutException {
        if (entry.isEmpty()) {
            throw new InvalidUnknownFileException("Invalid Unknown File");
        }

        if (new File(entry).isAbsolute()) {
            throw new RootUnknownFileException("Absolute Unknown Files is not allowed");
        }

        final String canonicalDirPath = directory.getCanonicalPath() + File.separator;
        final String canonicalEntryPath = new File(directory, entry).getCanonicalPath();

        if (!canonicalEntryPath.startsWith(canonicalDirPath)) {
            throw new TraversalUnknownFileException("Directory Traversal is not allowed");
        }

        // https://stackoverflow.com/q/2375903/455008
        return canonicalEntryPath.substring(canonicalDirPath.length());
    }

my temporary solution

Looking forward to the perfect solution.

    public static String sanitizeFilepath(final File directory, final String entry) throws IOException, BrutException {
        if (entry.isEmpty()) {
            throw new InvalidUnknownFileException("Invalid Unknown File");
        }

        if (new File(entry).isAbsolute()) {
            throw new RootUnknownFileException("Absolute Unknown Files is not allowed");
        }

        final String canonicalDirPath = directory.getCanonicalPath() + File.separator;
        final File file = new File(directory, entry);
        String canonicalEntryPath = file.getCanonicalPath();
        if (canonicalEntryPath.contains("/assets/Assets/")) {
            final String absolutePath = file.getAbsolutePath();
            if (!canonicalEntryPath.equals(absolutePath)) {
                LOGGER.info("sanitizeFilepath: replace path, from = " + canonicalEntryPath + " , to = " + absolutePath);
                canonicalEntryPath = absolutePath;
            }
        }

        if (!canonicalEntryPath.startsWith(canonicalDirPath)) {
            throw new TraversalUnknownFileException("Directory Traversal is not allowed");
        }

        // https://stackoverflow.com/q/2375903/455008
        return canonicalEntryPath.substring(canonicalDirPath.length());
    }
@iBotPeaches
Copy link
Owner

Thanks for the report. May you submit a failing test case to the repo? I've seen a few iterations of this bug and never really track it down.

Since I can't really accept a patch that just hard-codes assets/Assets - it must be some case sensitive changes we can do?

@yangbo637829
Copy link
Author

Thanks for the report. May you submit a failing test case to the repo? I've seen a few iterations of this bug and never really track it down.

Since I can't really accept a patch that just hard-codes assets/Assets - it must be some case sensitive changes we can do?

    @Test
    public void validAssetsFileTest() throws IOException, BrutException {
        File asset = new File(sTmpDir, "assets");
        if (!asset.exists()) {
            asset.mkdirs();
        }
        File Asset = new File(asset, "Assets");
        if (!Asset.exists()) {
            Asset.mkdirs();
        }
        TestUtils.copyResourceDir(UnknownDirectoryTraversalTest.class, "util/traversal", asset);

        String validFilename = BrutIO.sanitizeFilepath(asset, "assets/file");
        assertEquals(validFilename, "assets/file");
    }
image

@iBotPeaches
Copy link
Owner

thanks. I'll take a look with this sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants