From a4bf364d29b78f4e197b67cc1e8f06dc0e3dd9c4 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Wed, 27 Jul 2022 16:35:43 +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 --- ...CollectIlluminaBasecallingMetricsTest.java | 7 +++-- .../illumina/ExtractIlluminaBarcodesTest.java | 31 ++++++++++--------- .../IlluminaBasecallsToFastqTest.java | 9 ++---- .../metrics/CollectRrbsMetricsTest.java | 7 +++-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/test/java/picard/illumina/CollectIlluminaBasecallingMetricsTest.java b/src/test/java/picard/illumina/CollectIlluminaBasecallingMetricsTest.java index 2b269c9991..1adc35704f 100644 --- 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 index 049792d548..963310afa8 100644 --- 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 index 7189acfefa..debd23ada5 100644 --- 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 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 index c84a04b703..c2d4c8202e 100644 --- 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