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]>

Author: JLLeitschuh

src/main/java/com/salesforce/perfeng/uiperf/imageoptimization/service/ImageOptimizationService.java

@@ -346,9 +346,7 @@ public final static <C> ImageOptimizationService<C> createInstance(final String
             logger.debug("Current local directory is: {}", new File(".").getCanonicalPath());
         }
 
-        final File tmpDir = File.createTempFile(ImageOptimizationService.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        final File tmpDir = Files.createTempDirectory(ImageOptimizationService.class.getName()).toFile();
         return new ImageOptimizationService<>(tmpDir, new File(pathToBinaryProgramsForImageOptimizationDirectory).getCanonicalFile(), timeoutInSeconds);
     }
 

src/test/java/com/salesforce/perfeng/uiperf/imageoptimization/service/ImageOptimizationServiceTest.java

@@ -44,6 +44,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -91,9 +92,7 @@ public class ImageOptimizationServiceTest {
      */
     @BeforeEach
     public void setUp() throws IOException {
-        final File tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        final File tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
 
         imageOptimizationService = new ImageOptimizationService<>(tmpDir, new File(DEFAULT_BINARY_APP_LOCATION));
@@ -120,9 +119,7 @@ public void testImageOptimizationService() throws IOException {
         actualException = assertThrows(IllegalArgumentException.class, () -> new ImageOptimizationService<>(file, new File(DEFAULT_BINARY_APP_LOCATION)));
         assertThat(actualException.getMessage(), matchesRegex("The passed in tmpWorkingDirectory, \".+\", needs to be a directory."));
 
-        final File tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        final File tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
         assertThat(new ImageOptimizationService<>(tmpDir, new File(DEFAULT_BINARY_APP_LOCATION)), notNullValue());
     }
@@ -150,9 +147,7 @@ public void testImageOptimizationService2() throws IOException {
         actualException = assertThrows(IllegalArgumentException.class, () -> new ImageOptimizationService<>(file, new File(DEFAULT_BINARY_APP_LOCATION), 1));
         assertThat(actualException.getMessage(), matchesRegex("The passed in tmpWorkingDirectory, \".+\", needs to be a directory."));
 
-        File tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        File tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
         assertThat(new ImageOptimizationService<>(tmpDir, new File(DEFAULT_BINARY_APP_LOCATION), 1), notNullValue());
 
@@ -166,9 +161,7 @@ public void testImageOptimizationService2() throws IOException {
         file2.deleteOnExit();
         actualException = assertThrows(IllegalArgumentException.class, () -> new ImageOptimizationService<>(file2, new File(DEFAULT_BINARY_APP_LOCATION), 0));
 
-        tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
         assertThat(new ImageOptimizationService<>(tmpDir, new File(DEFAULT_BINARY_APP_LOCATION), 0), notNullValue());
     }
@@ -222,9 +215,7 @@ private static final void validateFileOptimization(final OptimizationResult<Obje
     }
 
     private static final File getTempDir() throws IOException {
-        final File tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        final File tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
         return tmpDir;
     }
@@ -1188,4 +1179,4 @@ public boolean equals(final Object obj) {
                     && masterFileChecksum == other.masterFileChecksum;
         }
     }
-}
+}

src/test/java/com/salesforce/perfeng/uiperf/imageoptimization/utils/ImageUtilsTest.java

@@ -46,6 +46,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URLConnection;
+import java.nio.file.Files;
 
 import org.apache.commons.io.FileUtils;
 import org.hamcrest.io.FileMatchers;
@@ -163,9 +164,7 @@ public void testConvertImageNative() throws IOException, ThirdPartyBinaryNotFoun
         }
         assertThat(ImageUtils.visuallyCompare(new File("./src/test/java/com/salesforce/perfeng/uiperf/imageoptimization/utils/forceapp_bg.gif"), convertedFile), equalTo(TRUE));
 
-        final File tmpDir = File.createTempFile(ImageOptimizationServiceTest.class.getName(), "");
-        tmpDir.delete();
-        tmpDir.mkdir();
+        final File tmpDir = Files.createTempDirectory(ImageOptimizationServiceTest.class.getName()).toFile();
         tmpDir.deleteOnExit();
 
         convertedFile = new File(tmpDir.getCanonicalPath() + "/forceapp_bg." + IImageOptimizationService.PNG_EXTENSION);
@@ -195,4 +194,4 @@ public void testIsAminatedGif() {
         assertThat(ImageUtils.isAminatedGif(new File("./src/test/java/com/salesforce/perfeng/uiperf/imageoptimization/service/el_icon.gif")), equalTo(FALSE));
         assertThat(ImageUtils.isAminatedGif(new File("./src/test/java/com/salesforce/perfeng/uiperf/imageoptimization/service/addCol.gif")), equalTo(FALSE));
     }
-}
+}