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 · a4bf364 · a4bf364
1 parent c97fce5
commit a4bf364
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 27 deletions.
diff --git a/src/test/java/picard/illumina/CollectIlluminaBasecallingMetricsTest.java b/src/test/java/picard/illumina/CollectIlluminaBasecallingMetricsTest.java
@@ -10,6 +10,7 @@
 
 import java.io.File;
 import java.io.FileReader;
+import java.nio.file.Files;
 import java.util.ArrayList;
 
 public class CollectIlluminaBasecallingMetricsTest {
@@ -19,9 +20,9 @@ public class CollectIlluminaBasecallingMetricsTest {
 
     @BeforeTest
     private void setUp() throws Exception {
-        rootTestDir = File.createTempFile("cibm.", ".tmp");
-        Assert.assertTrue(rootTestDir.delete());
-        Assert.assertTrue(rootTestDir.mkdir());
+        rootTestDir = Files.createTempDirectory("cibm." + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         for (final File source : TEST_DATA_DIR.listFiles()) {
             if (source.isDirectory() && !source.isHidden()) {
                 IOUtil.copyDirectoryTree(source, new File(rootTestDir.getPath(),source.getName()));

diff --git a/src/test/java/picard/illumina/ExtractIlluminaBarcodesTest.java b/src/test/java/picard/illumina/ExtractIlluminaBarcodesTest.java
@@ -39,6 +39,7 @@
 
 import java.io.File;
 import java.io.FileReader;
+import java.nio.file.Files;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
@@ -79,29 +80,29 @@ public String getCommandLineProgramName() {
 
     @BeforeTest
     private void setUp() throws Exception {
-        basecallsDir = File.createTempFile("eib.", ".tmp");
-        Assert.assertTrue(basecallsDir.delete());
-        Assert.assertTrue(basecallsDir.mkdir());
+        basecallsDir = Files.createTempDirectory("eib." + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         IOUtil.copyDirectoryTree(SINGLE_DATA_DIR, basecallsDir);
 
-        dual = File.createTempFile("eib_dual", ".tmp");
-        Assert.assertTrue(dual.delete());
-        Assert.assertTrue(dual.mkdir());
+        dual = Files.createTempDirectory("eib_dual" + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         IOUtil.copyDirectoryTree(DUAL_DATA_DIR, dual);
 
-        qual = File.createTempFile("eib_qual", ".tmp");
-        Assert.assertTrue(qual.delete());
-        Assert.assertTrue(qual.mkdir());
+        qual = Files.createTempDirectory("eib_qual" + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         IOUtil.copyDirectoryTree(DUAL_DATA_DIR, qual);
 
-        noSymlink = File.createTempFile("eib_nosymlink", ".tmp");
-        Assert.assertTrue(noSymlink.delete());
-        Assert.assertTrue(noSymlink.mkdir());
+        noSymlink = Files.createTempDirectory("eib_nosymlink" + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         IOUtil.copyDirectoryTree(HISEQX_DATA_DIR, noSymlink);
 
-        cbcl = File.createTempFile("eib_cbcl", ".tmp");
-        Assert.assertTrue(cbcl.delete());
-        Assert.assertTrue(cbcl.mkdir());
+        cbcl = Files.createTempDirectory("eib_cbcl" + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
         IOUtil.copyDirectoryTree(CBCL_DATA_DIR, cbcl);
         // For the cbcl test, we are deleting the '*barcode.txt.gz' files that exist in the test Basecalls directory
         // This is to prevent the error conditon that was briefly introduced which expected to find such files in that

diff --git a/src/test/java/picard/illumina/IlluminaBasecallsToFastqTest.java b/src/test/java/picard/illumina/IlluminaBasecallsToFastqTest.java
@@ -34,6 +34,7 @@
 import picard.util.IlluminaUtil;
 
 import java.io.*;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
@@ -185,10 +186,8 @@ public void testQualityAndAdapterTrimming() throws Exception {
 
     @Test
     public void testMultiplexWithIlluminaReadNameHeaders() throws Exception {
-        final File outputDir = File.createTempFile("testMultiplexRH.", ".dir");
+        final File outputDir = Files.createTempDirectory("testMultiplexRH." + ".dir").toFile();
         try {
-            outputDir.delete();
-            outputDir.mkdir();
             outputDir.deleteOnExit();
 
             final String filePrefix = "testMultiplexRH";
@@ -290,10 +289,8 @@ private List<FastqRecord> slurpReads(final File f) {
     private void runStandardTest(final int[] lanes, final String jobName, final String libraryParamsFile,
                                  final int concatNColumnFields, final String readStructureString, final File baseCallsDir,
                                  final File testDataDir, final long expectedPfMatches, final Double expectedPctMatches) throws Exception {
-        final File outputDir = File.createTempFile(jobName, ".dir");
+        final File outputDir = Files.createTempDirectory(jobName + ".dir").toFile();
         try {
-            outputDir.delete();
-            outputDir.mkdir();
 
             outputDir.deleteOnExit();
             // Create barcode.params with output files in the temp directory

diff --git a/src/test/java/picard/metrics/CollectRrbsMetricsTest.java b/src/test/java/picard/metrics/CollectRrbsMetricsTest.java
@@ -36,6 +36,7 @@
 
 import java.io.File;
 import java.io.FileReader;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -50,9 +51,9 @@ public class CollectRrbsMetricsTest extends CommandLineProgramTest {
 
     @BeforeTest
     private void setUp() throws Exception {
-        rootTestDir = File.createTempFile("crmt.", ".tmp");
-        Assert.assertTrue(rootTestDir.delete());
-        Assert.assertTrue(rootTestDir.mkdir());
+        rootTestDir = Files.createTempDirectory("crmt." + ".tmp").toFile();
+        Assert.assertTrue(true);
+        Assert.assertTrue(true);
     }
 
     @AfterTest