From 9f36ac000dc31eb49169b435beea4281683b0a1c Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Wed, 27 Jul 2022 16:35:52 +0000 Subject: [PATCH] vuln-fix: Temporary Directory Hijacking or Information Disclosure This fixes either Temporary Directory Hijacking, or Temporary Directory Local Information Disclosure. Weakness: CWE-379: Creation of Temporary File in Directory with Insecure Permissions Severity: High CVSSS: 7.3 Detection: CodeQL & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.UseFilesCreateTempDirectory) Reported-by: Jonathan Leitschuh Signed-off-by: Jonathan Leitschuh Bug-tracker: https://github.com/JLLeitschuh/security-research/issues/10 Co-authored-by: Moderne --- .../zip/FilePermissionsTest.java | 13 +++---- .../zeroturnaround/zip/MainExamplesTest.java | 9 ++--- .../org/zeroturnaround/zip/ZipUtilTest.java | 35 +++++-------------- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/src/test/java/org/zeroturnaround/zip/FilePermissionsTest.java b/src/test/java/org/zeroturnaround/zip/FilePermissionsTest.java index c8b0aaf..b825a8b 100644 --- a/src/test/java/org/zeroturnaround/zip/FilePermissionsTest.java +++ b/src/test/java/org/zeroturnaround/zip/FilePermissionsTest.java @@ -16,6 +16,7 @@ */ import java.io.File; import java.io.IOException; +import java.nio.file.Files; import org.junit.Assume; import org.zeroturnaround.zip.commons.FileUtils; @@ -31,9 +32,7 @@ public class FilePermissionsTest { public void testPreserveExecuteFlag() throws Exception { String dirName = "FilePermissionsTest-e"; - File tmpDir = File.createTempFile(dirName, null); - tmpDir.delete(); - tmpDir.mkdir(); + File tmpDir = Files.createTempDirectory(dirName).toFile(); File fileA = new File(tmpDir, "fileA.txt"); File fileB = new File(tmpDir, "fileB.txt"); FileUtils.copyFile(testFile, fileA); @@ -62,9 +61,7 @@ public void testPreserveExecuteFlag() throws Exception { public void testPreserveReadFlag() throws Exception { String dirName = "FilePermissionsTest-r"; - File tmpDir = File.createTempFile(dirName, null); - tmpDir.delete(); - tmpDir.mkdir(); + File tmpDir = Files.createTempDirectory(dirName).toFile(); File fileA = new File(tmpDir, "fileA.txt"); File fileB = new File(tmpDir, "fileB.txt"); FileUtils.copyFile(testFile, fileA); @@ -95,9 +92,7 @@ public void testPreserveReadFlag() throws Exception { public void testPreserveWriteFlag() throws Exception { String dirName = "FilePermissionsTest-w"; - File tmpDir = File.createTempFile(dirName, null); - tmpDir.delete(); - tmpDir.mkdir(); + File tmpDir = Files.createTempDirectory(dirName).toFile(); File fileA = new File(tmpDir, "fileA.txt"); File fileB = new File(tmpDir, "fileB.txt"); FileUtils.copyFile(testFile, fileA, true); diff --git a/src/test/java/org/zeroturnaround/zip/MainExamplesTest.java b/src/test/java/org/zeroturnaround/zip/MainExamplesTest.java index 57ddfaf..8304485 100644 --- a/src/test/java/org/zeroturnaround/zip/MainExamplesTest.java +++ b/src/test/java/org/zeroturnaround/zip/MainExamplesTest.java @@ -18,6 +18,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import java.util.zip.ZipEntry; import junit.framework.TestCase; @@ -52,9 +53,7 @@ public static void testUnpackEntry() throws IOException { } public static void testUnpack() throws IOException { - File tmpDir = File.createTempFile("prefix", "suffix"); - tmpDir.delete(); - tmpDir.mkdir(); + File tmpDir = Files.createTempDirectory("prefix" + "suffix").toFile(); ZipUtil.unpack(new File(DEMO_ZIP), tmpDir); File fooFile = new File(tmpDir, FOO_TXT); assertTrue(fooFile.exists()); @@ -62,9 +61,7 @@ public static void testUnpack() throws IOException { public static void testUnpackInPlace() throws Exception{ File demoFile = new File(DEMO_ZIP); - File outDir = File.createTempFile("prefix", "suffix"); - outDir.delete(); - outDir.mkdir(); + File outDir = Files.createTempDirectory("prefix" + "suffix").toFile(); File outFile = new File(outDir, "demo"); FileOutputStream fio = new FileOutputStream(outFile); diff --git a/src/test/java/org/zeroturnaround/zip/ZipUtilTest.java b/src/test/java/org/zeroturnaround/zip/ZipUtilTest.java index e691c05..79c876c 100644 --- a/src/test/java/org/zeroturnaround/zip/ZipUtilTest.java +++ b/src/test/java/org/zeroturnaround/zip/ZipUtilTest.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -687,10 +688,8 @@ public void process(InputStream in, ZipEntry zipEntry) throws IOException { public void testUnwrapFile() throws Exception { File dest = File.createTempFile("temp", null); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); try { - destDir.delete(); - destDir.mkdir(); String child = "TestFile.txt"; File parent = file(child).getParentFile(); ZipUtil.pack(parent, dest, true); @@ -704,11 +703,9 @@ public void testUnwrapFile() throws Exception { public void testUnwrapStream() throws Exception { File dest = File.createTempFile("temp", null); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); InputStream is = null; try { - destDir.delete(); - destDir.mkdir(); String child = "TestFile.txt"; File parent = file(child).getParentFile(); ZipUtil.pack(parent, dest, true); @@ -724,10 +721,8 @@ public void testUnwrapStream() throws Exception { public void testUnwrapEntriesInRoot() throws Exception { File src = file("demo.zip"); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); try { - destDir.delete(); - destDir.mkdir(); ZipUtil.unwrap(src, destDir); fail("expected a ZipException, unwrapping with multiple roots is not supported"); } @@ -741,10 +736,8 @@ public void testUnwrapEntriesInRoot() throws Exception { public void testUnwrapMultipleRoots() throws Exception { File src = file("demo-dirs-only.zip"); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); try { - destDir.delete(); - destDir.mkdir(); ZipUtil.unwrap(src, destDir); fail("expected a ZipException, unwrapping with multiple roots is not supported"); } @@ -758,10 +751,8 @@ public void testUnwrapMultipleRoots() throws Exception { public void testUnwrapSingleRootWithStructure() throws Exception { File src = file("demo-single-root-dir.zip"); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); try { - destDir.delete(); - destDir.mkdir(); ZipUtil.unwrap(src, destDir); assertTrue((new File(destDir, "b.txt")).exists()); assertTrue((new File(destDir, "bad.txt")).exists()); @@ -775,10 +766,8 @@ public void testUnwrapSingleRootWithStructure() throws Exception { public void testUnwrapEmptyRootDir() throws Exception { File src = file("demo-single-empty-root-dir.zip"); - File destDir = File.createTempFile("tempDir", null); + File destDir = Files.createTempDirectory("tempDir").toFile(); try { - destDir.delete(); - destDir.mkdir(); ZipUtil.unwrap(src, destDir); assertTrue("Dest dir should be empty, root dir was shaved", destDir.list().length == 0); } @@ -885,15 +874,7 @@ public void testUnpackBackslashes() throws IOException { File initialSrc = file("backSlashTest.zip"); // lets create a temporary file and then use it as a dir - File dest = File.createTempFile("unpackEntryDir", null); - - if(!(dest.delete())) { - throw new IOException("Could not delete temp file: " + dest.getAbsolutePath()); - } - - if(!(dest.mkdir())){ - throw new IOException("Could not create temp directory: " + dest.getAbsolutePath()); - } + File dest = Files.createTempDirectory("unpackEntryDir").toFile(); // unpack the archive that has the backslashes // and double check that the file structure is preserved