mirror of
https://github.com/revanced/Apktool.git
synced 2024-09-01 12:05:31 +02:00
Prevent arbitrary file writes with malicious resource names. (#3484)
* refactor: rename sanitize function * fix: expose getDir * fix: safe handling of untrusted resource names - fixes: GHSA-2hqv-2xv4-5h5w * test: sample file for GHSA-2hqv-2xv4-5h5w * refactor: avoid detection of absolute files for resource check * chore: enable info mode on gradle * test: skip test on windows * chore: debug windows handling * fix: normalize entry with file separators * fix: normalize filepath after cleansing * chore: Android paths are not OS specific * refactor: use java.nio for path traversal checking * chore: align path separator on Windows for Zip files * chore: rework towards basic directory traversal * chore: remove '--info' on build.yml
This commit is contained in:
parent
077b2009da
commit
93e7d6bdbf
@ -482,7 +482,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;
|
||||
|
@ -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();
|
||||
|
@ -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;
|
||||
|
@ -0,0 +1,65 @@
|
||||
/*
|
||||
* Copyright (C) 2010 Ryszard Wiśniewski <brut.alll@gmail.com>
|
||||
* Copyright (C) 2010 Connor Tumbleson <connor.tumbleson@gmail.com>
|
||||
*
|
||||
* 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());
|
||||
}
|
||||
}
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
Binary file not shown.
@ -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
|
||||
|
@ -119,7 +119,7 @@ public class FileDirectory extends AbstractDirectory {
|
||||
}
|
||||
}
|
||||
|
||||
private File getDir() {
|
||||
public File getDir() {
|
||||
return mDir;
|
||||
}
|
||||
}
|
||||
|
@ -59,8 +59,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
|
||||
|
@ -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 != '/') {
|
||||
|
Loading…
Reference in New Issue
Block a user