From 85acbc91d6d14fc4273416b63de9fee1a49e729f Mon Sep 17 00:00:00 2001 From: Dan Rosser Date: Sat, 15 Aug 2026 14:25:39 +1000 Subject: [PATCH] Fix Android ZIP path traversal --- .../Java/cc/openframeworks/OFZipUtil.java | 50 ++++++++++--- tests/android/OFZipUtil/run.sh | 13 ++++ .../android/OFZipUtil/src/OFZipUtilTest.java | 74 +++++++++++++++++++ tests/android/OFZipUtil/src/main.cpp | 5 ++ .../OFZipUtil/stubs/android/util/Log.java | 9 +++ .../stubs/androidx/annotation/Keep.java | 5 ++ 6 files changed, 146 insertions(+), 10 deletions(-) create mode 100755 tests/android/OFZipUtil/run.sh create mode 100644 tests/android/OFZipUtil/src/OFZipUtilTest.java create mode 100644 tests/android/OFZipUtil/src/main.cpp create mode 100644 tests/android/OFZipUtil/stubs/android/util/Log.java create mode 100644 tests/android/OFZipUtil/stubs/androidx/annotation/Keep.java diff --git a/addons/ofxAndroid/Java/cc/openframeworks/OFZipUtil.java b/addons/ofxAndroid/Java/cc/openframeworks/OFZipUtil.java index bf288d0f591..d3b64780a45 100644 --- a/addons/ofxAndroid/Java/cc/openframeworks/OFZipUtil.java +++ b/addons/ofxAndroid/Java/cc/openframeworks/OFZipUtil.java @@ -16,21 +16,49 @@ public class OFZipUtil { private static final int BUFFER_SIZE = 4096; + private static File resolveEntry(File outdir, String name) throws IOException + { + File canonicalOutdir = outdir.getCanonicalFile(); + File destination = new File(canonicalOutdir, name).getCanonicalFile(); + String outdirPath = canonicalOutdir.getPath(); + String destinationPath = destination.getPath(); + + if (!destinationPath.equals(outdirPath) + && !destinationPath.startsWith(outdirPath + File.separator)) + throw new IOException("Zip entry is outside the target directory: " + name); + + return destination; + } + public static void extractFile(ZipInputStream in, File outdir, String name) throws IOException { byte[] buffer = new byte[BUFFER_SIZE]; - BufferedOutputStream out = new BufferedOutputStream(new FileOutputStream(new File(outdir,name))); - int count = -1; - while ((count = in.read(buffer)) != -1) - out.write(buffer, 0, count); - out.close(); + File destination = resolveEntry(outdir, name); + try (BufferedOutputStream out = new BufferedOutputStream(new FileOutputStream(destination))) + { + int count; + while ((count = in.read(buffer)) != -1) + out.write(buffer, 0, count); + } + } + + private static void createDirectories(File outdir, String path) throws IOException + { + File directory = resolveEntry(outdir, path); + if (!directory.exists() && !directory.mkdirs() && !directory.isDirectory()) + throw new IOException("Could not create directory: " + directory); } public static void mkdirs(File outdir,String path) { - File d = new File(outdir, path); - if( !d.exists() ) - d.mkdirs(); + try + { + createDirectories(outdir, path); + } + catch (IOException e) + { + throw new IllegalArgumentException(e); + } } public static String dirpart(String name) @@ -55,9 +83,11 @@ public static void extract(InputStream zipfile, File outdir) while ((entry = zin.getNextEntry()) != null) { name = entry.getName(); + // Validate every entry before creating directories or opening files. + resolveEntry(outdir, name); if( entry.isDirectory() ) { - mkdirs(outdir,name); + createDirectories(outdir,name); continue; } /* this part is necessary because file entry can come before @@ -68,7 +98,7 @@ public static void extract(InputStream zipfile, File outdir) */ dir = dirpart(name); if( dir != null ) - mkdirs(outdir,dir); + createDirectories(outdir,dir); extractFile(zin, outdir, name); } diff --git a/tests/android/OFZipUtil/run.sh b/tests/android/OFZipUtil/run.sh new file mode 100755 index 00000000000..8220e56e8de --- /dev/null +++ b/tests/android/OFZipUtil/run.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" +TEST_CLASSES="$(mktemp -d)" + +javac -d "$TEST_CLASSES" \ + "$REPO_ROOT/tests/android/OFZipUtil/stubs/android/util/Log.java" \ + "$REPO_ROOT/tests/android/OFZipUtil/stubs/androidx/annotation/Keep.java" \ + "$REPO_ROOT/addons/ofxAndroid/Java/cc/openframeworks/OFZipUtil.java" \ + "$REPO_ROOT/tests/android/OFZipUtil/src/OFZipUtilTest.java" + +java -cp "$TEST_CLASSES" OFZipUtilTest diff --git a/tests/android/OFZipUtil/src/OFZipUtilTest.java b/tests/android/OFZipUtil/src/OFZipUtilTest.java new file mode 100644 index 00000000000..a5b7b81f16b --- /dev/null +++ b/tests/android/OFZipUtil/src/OFZipUtilTest.java @@ -0,0 +1,74 @@ +import cc.openframeworks.OFZipUtil; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +public class OFZipUtilTest +{ + private static byte[] zip(String name, String contents, boolean directory) throws Exception + { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ZipOutputStream zip = new ZipOutputStream(bytes)) + { + ZipEntry entry = new ZipEntry(name); + zip.putNextEntry(entry); + if (!directory) + zip.write(contents.getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + return bytes.toByteArray(); + } + + private static void extract(File output, String name, String contents) throws Exception + { + OFZipUtil.extract(new ByteArrayInputStream(zip(name, contents, false)), output); + } + + private static void assertMissing(Path path, String message) + { + if (Files.exists(path)) + throw new AssertionError(message + ": " + path); + } + + public static void main(String[] args) throws Exception + { + Path parent = Files.createTempDirectory("ofziputil-test-"); + File output = Files.createDirectory(parent.resolve("output")).toFile(); + + extract(output, "nested/good.txt", "good"); + Path good = output.toPath().resolve("nested/good.txt"); + if (!Files.isRegularFile(good) + || !"good".equals(Files.readString(good, StandardCharsets.UTF_8))) + throw new AssertionError("A safe nested entry was not extracted correctly"); + + Path escaped = parent.resolve("escaped.txt"); + extract(output, "../escaped.txt", "bad"); + assertMissing(escaped, "A parent traversal entry escaped the output directory"); + + extract(output, "nested/../../escaped.txt", "bad"); + assertMissing(escaped, "A nested parent traversal entry escaped the output directory"); + + Path absolute = parent.resolve("absolute.txt"); + extract(output, absolute.toString(), "bad"); + assertMissing(absolute, "An absolute entry escaped the output directory"); + + Path escapedDirectory = parent.resolve("escaped-directory"); + OFZipUtil.extract( + new ByteArrayInputStream(zip("../escaped-directory/", "", true)), output); + assertMissing(escapedDirectory, "A directory entry escaped the output directory"); + + Path outside = Files.createDirectory(parent.resolve("outside")); + Files.createSymbolicLink(output.toPath().resolve("link"), outside); + extract(output, "link/escaped.txt", "bad"); + assertMissing(outside.resolve("escaped.txt"), + "An entry escaped through a symbolic link in the output directory"); + + System.out.println("OFZipUtil security regression tests passed"); + } +} diff --git a/tests/android/OFZipUtil/src/main.cpp b/tests/android/OFZipUtil/src/main.cpp new file mode 100644 index 00000000000..6b908ae8cf1 --- /dev/null +++ b/tests/android/OFZipUtil/src/main.cpp @@ -0,0 +1,5 @@ +#ifndef TARGET_ANDROID +int main() { + return 0; +} +#endif diff --git a/tests/android/OFZipUtil/stubs/android/util/Log.java b/tests/android/OFZipUtil/stubs/android/util/Log.java new file mode 100644 index 00000000000..734176fc505 --- /dev/null +++ b/tests/android/OFZipUtil/stubs/android/util/Log.java @@ -0,0 +1,9 @@ +package android.util; + +public class Log +{ + public static int i(String tag, String message) + { + return 0; + } +} diff --git a/tests/android/OFZipUtil/stubs/androidx/annotation/Keep.java b/tests/android/OFZipUtil/stubs/androidx/annotation/Keep.java new file mode 100644 index 00000000000..d2476568397 --- /dev/null +++ b/tests/android/OFZipUtil/stubs/androidx/annotation/Keep.java @@ -0,0 +1,5 @@ +package androidx.annotation; + +public @interface Keep +{ +}