diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/ApkBuilder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/ApkBuilder.java index 12153e25..5c9e3de4 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/ApkBuilder.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/ApkBuilder.java @@ -480,7 +480,7 @@ public class ApkBuilder { File inputFile; try { - inputFile = new File(unknownFileDir, BrutIO.sanitizeUnknownFile(unknownFileDir, unknownFileInfo.getKey())); + inputFile = new File(unknownFileDir, BrutIO.sanitizeFilepath(unknownFileDir, unknownFileInfo.getKey())); } catch (RootUnknownFileException | InvalidUnknownFileException | TraversalUnknownFileException exception) { LOGGER.warning(String.format("Skipping file %s (%s)", unknownFileInfo.getKey(), exception.getMessage())); continue; diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/ResourcesDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/ResourcesDecoder.java index a9507b6b..7ee7f77e 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/ResourcesDecoder.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/ResourcesDecoder.java @@ -154,12 +154,12 @@ public class ResourcesDecoder { decoders.setDecoder("xml", new XmlPullStreamDecoder(axmlParser, getResXmlSerializer())); ResFileDecoder fileDecoder = new ResFileDecoder(decoders); - Directory in, out; + Directory in, out, outRes; try { out = new FileDirectory(outDir); in = mApkInfo.getApkFile().getDirectory(); - out = out.createDir("res"); + outRes = out.createDir("res"); } catch (DirectoryException ex) { throw new AndrolibException(ex); } @@ -169,14 +169,14 @@ public class ResourcesDecoder { LOGGER.info("Decoding file-resources..."); for (ResResource res : pkg.listFiles()) { - fileDecoder.decode(res, in, out, mResFileMapping); + fileDecoder.decode(res, in, outRes, mResFileMapping); } LOGGER.info("Decoding values */* XMLs..."); for (ResValuesFile valuesFile : pkg.listValuesFiles()) { - generateValuesFile(valuesFile, out, xmlSerializer); + generateValuesFile(valuesFile, outRes, xmlSerializer); } - generatePublicXml(pkg, out, xmlSerializer); + generatePublicXml(pkg, outRes, xmlSerializer); } AndrolibException decodeError = axmlParser.getFirstError(); diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ResFileDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ResFileDecoder.java index 9bab7c98..979d89a8 100644 --- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ResFileDecoder.java +++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ResFileDecoder.java @@ -25,6 +25,7 @@ import brut.androlib.res.data.value.ResFileValue; import brut.directory.DirUtil; import brut.directory.Directory; import brut.directory.DirectoryException; +import brut.util.BrutIO; import java.io.*; import java.util.Map; @@ -44,8 +45,15 @@ public class ResFileDecoder { ResFileValue fileValue = (ResFileValue) res.getValue(); String inFilePath = fileValue.toString(); String inFileName = fileValue.getStrippedPath(); - String outResName = res.getFilePath(); String typeName = res.getResSpec().getType().getName(); + String outResName = res.getFilePath(); + + if (BrutIO.detectPossibleDirectoryTraversal(outResName)) { + outResName = inFileName; + LOGGER.warning(String.format( + "Potentially malicious file path: %s, using instead %s", res.getFilePath(), outResName + )); + } String ext = null; String outFileName; diff --git a/brut.apktool/apktool-lib/src/test/java/brut/androlib/decode/ResourceDirectoryTraversalTest.java b/brut.apktool/apktool-lib/src/test/java/brut/androlib/decode/ResourceDirectoryTraversalTest.java new file mode 100644 index 00000000..7d308db5 --- /dev/null +++ b/brut.apktool/apktool-lib/src/test/java/brut/androlib/decode/ResourceDirectoryTraversalTest.java @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2010 Ryszard Wiśniewski + * Copyright (C) 2010 Connor Tumbleson + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package brut.androlib.decode; + +import brut.androlib.ApkDecoder; +import brut.androlib.BaseTest; +import brut.androlib.Config; +import brut.androlib.TestUtils; +import brut.common.BrutException; +import brut.directory.ExtFile; +import brut.util.OS; +import brut.util.OSDetection; +import org.junit.AfterClass; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; + +import static org.junit.Assert.assertTrue; + +public class ResourceDirectoryTraversalTest extends BaseTest { + + @BeforeClass + public static void beforeClass() throws Exception { + TestUtils.cleanFrameworkFile(); + sTmpDir = new ExtFile(OS.createTempDirectory()); + TestUtils.copyResourceDir(ResourceDirectoryTraversalTest.class, "decode/arbitrary-write/", sTmpDir); + Assume.assumeFalse(OSDetection.isWindows()); + } + + @AfterClass + public static void afterClass() throws BrutException { + OS.rmdir(sTmpDir); + } + + @Test + public void checkIfMaliciousRawFileIsDisassembledProperly() throws BrutException, IOException { + String apk = "GHSA-2hqv-2xv4-5h5w.apk"; + + Config config = Config.getDefaultConfig(); + config.forceDelete = true; + ApkDecoder apkDecoder = new ApkDecoder(config, new File(sTmpDir + File.separator + apk)); + File outDir = new File(sTmpDir + File.separator + apk + ".out"); + apkDecoder.decode(outDir); + + File pocTestFile = new File(outDir,"res/raw/poc"); + assertTrue(pocTestFile.exists()); + } +} diff --git a/brut.apktool/apktool-lib/src/test/java/brut/androlib/util/UnknownDirectoryTraversalTest.java b/brut.apktool/apktool-lib/src/test/java/brut/androlib/util/UnknownDirectoryTraversalTest.java index ad22e9d0..269b3fc0 100644 --- a/brut.apktool/apktool-lib/src/test/java/brut/androlib/util/UnknownDirectoryTraversalTest.java +++ b/brut.apktool/apktool-lib/src/test/java/brut/androlib/util/UnknownDirectoryTraversalTest.java @@ -51,7 +51,7 @@ public class UnknownDirectoryTraversalTest extends BaseTest { @Test public void validFileTest() throws IOException, BrutException { - String validFilename = BrutIO.sanitizeUnknownFile(sTmpDir, "file"); + String validFilename = BrutIO.sanitizeFilepath(sTmpDir, "file"); assertEquals(validFilename, "file"); File validFile = new File(sTmpDir, validFilename); @@ -60,18 +60,18 @@ public class UnknownDirectoryTraversalTest extends BaseTest { @Test(expected = TraversalUnknownFileException.class) public void invalidBackwardFileTest() throws IOException, BrutException { - BrutIO.sanitizeUnknownFile(sTmpDir, "../file"); + BrutIO.sanitizeFilepath(sTmpDir, "../file"); } @Test(expected = RootUnknownFileException.class) public void invalidRootFileTest() throws IOException, BrutException { String rootLocation = OSDetection.isWindows() ? "C:/" : File.separator; - BrutIO.sanitizeUnknownFile(sTmpDir, rootLocation + "file"); + BrutIO.sanitizeFilepath(sTmpDir, rootLocation + "file"); } @Test(expected = InvalidUnknownFileException.class) public void noFilePassedTest() throws IOException, BrutException { - BrutIO.sanitizeUnknownFile(sTmpDir, ""); + BrutIO.sanitizeFilepath(sTmpDir, ""); } @Test(expected = TraversalUnknownFileException.class) @@ -83,12 +83,12 @@ public class UnknownDirectoryTraversalTest extends BaseTest { invalidPath = "..\\..\\app.exe"; } - BrutIO.sanitizeUnknownFile(sTmpDir, invalidPath); + BrutIO.sanitizeFilepath(sTmpDir, invalidPath); } @Test public void validDirectoryFileTest() throws IOException, BrutException { - String validFilename = BrutIO.sanitizeUnknownFile(sTmpDir, "dir" + File.separator + "file"); + String validFilename = BrutIO.sanitizeFilepath(sTmpDir, "dir" + File.separator + "file"); assertEquals("dir" + File.separator + "file", validFilename); } } diff --git a/brut.apktool/apktool-lib/src/test/resources/decode/arbitrary-write/GHSA-2hqv-2xv4-5h5w.apk b/brut.apktool/apktool-lib/src/test/resources/decode/arbitrary-write/GHSA-2hqv-2xv4-5h5w.apk new file mode 100644 index 00000000..cc177ce3 Binary files /dev/null and b/brut.apktool/apktool-lib/src/test/resources/decode/arbitrary-write/GHSA-2hqv-2xv4-5h5w.apk differ diff --git a/brut.j.dir/src/main/java/brut/directory/DirUtil.java b/brut.j.dir/src/main/java/brut/directory/DirUtil.java index bfc1e0a1..906fdac2 100644 --- a/brut.j.dir/src/main/java/brut/directory/DirUtil.java +++ b/brut.j.dir/src/main/java/brut/directory/DirUtil.java @@ -89,7 +89,7 @@ public class DirUtil { } else if (!in.containsDir(fileName) && !in.containsFile(fileName)) { // Skip copies of directories/files not found. } else { - String cleanedFilename = BrutIO.sanitizeUnknownFile(out, fileName); + String cleanedFilename = BrutIO.sanitizeFilepath(out, fileName); if (! cleanedFilename.isEmpty()) { File outFile = new File(out, cleanedFilename); //noinspection ResultOfMethodCallIgnored diff --git a/brut.j.dir/src/main/java/brut/directory/FileDirectory.java b/brut.j.dir/src/main/java/brut/directory/FileDirectory.java index 554900bd..8a9f9915 100644 --- a/brut.j.dir/src/main/java/brut/directory/FileDirectory.java +++ b/brut.j.dir/src/main/java/brut/directory/FileDirectory.java @@ -119,7 +119,7 @@ public class FileDirectory extends AbstractDirectory { } } - private File getDir() { + public File getDir() { return mDir; } } diff --git a/brut.j.dir/src/main/java/brut/directory/ZipUtils.java b/brut.j.dir/src/main/java/brut/directory/ZipUtils.java index 7cd81471..db9dcdf3 100644 --- a/brut.j.dir/src/main/java/brut/directory/ZipUtils.java +++ b/brut.j.dir/src/main/java/brut/directory/ZipUtils.java @@ -57,8 +57,8 @@ public class ZipUtils { throws BrutException, IOException { for (final File file : folder.listFiles()) { if (file.isFile()) { - final String cleanedPath = BrutIO.sanitizeUnknownFile(folder, file.getPath().substring(prefixLength)); - final ZipEntry zipEntry = new ZipEntry(BrutIO.normalizePath(cleanedPath)); + final String cleanedPath = BrutIO.sanitizeFilepath(folder, file.getPath().substring(prefixLength)); + final ZipEntry zipEntry = new ZipEntry(BrutIO.adaptSeparatorToUnix(cleanedPath)); // aapt binary by default takes in parameters via -0 arsc to list extensions that shouldn't be // compressed. We will replicate that behavior diff --git a/brut.j.util/src/main/java/brut/util/BrutIO.java b/brut.j.util/src/main/java/brut/util/BrutIO.java index ede9a853..feecc129 100644 --- a/brut.j.util/src/main/java/brut/util/BrutIO.java +++ b/brut.j.util/src/main/java/brut/util/BrutIO.java @@ -74,8 +74,8 @@ public class BrutIO { return crc; } - public static String sanitizeUnknownFile(final File directory, final String entry) throws IOException, BrutException { - if (entry.length() == 0) { + public static String sanitizeFilepath(final File directory, final String entry) throws IOException, BrutException { + if (entry.isEmpty()) { throw new InvalidUnknownFileException("Invalid Unknown File"); } @@ -94,7 +94,14 @@ public class BrutIO { return canonicalEntryPath.substring(canonicalDirPath.length()); } - public static String normalizePath(String path) { + public static boolean detectPossibleDirectoryTraversal(String entry) { + if (OSDetection.isWindows()) { + return entry.contains("..\\") || entry.contains("\\.."); + } + return entry.contains("../") || entry.contains("/.."); + } + + public static String adaptSeparatorToUnix(String path) { char separator = File.separatorChar; if (separator != '/') {