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 <[email protected]> Signed-off-by: Jonathan Leitschuh <[email protected]> Bug-tracker: JLLeitschuh/security-research#10 Co-authored-by: Moderne <[email protected]>
JLLeitschuh · Jul 27, 2022 · 9f36ac0 · 9f36ac0
1 parent 9b08188
commit 9f36ac0
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 42 deletions.
diff --git 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
@@ -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,19 +53,15 @@ 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());
   }
 
   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
@@ -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