vuln-fix: Temporary File Information Disclosure (#18)

This fixes temporary file information disclosure vulnerability due to the use of the vulnerable `File.createTempFile()` method. The vulnerability is fixed by using the `Files.createTempFile()` method which sets the correct posix permissions. Weakness: CWE-377: Insecure Temporary File Severity: Medium CVSSS: 5.5 Detection: CodeQL & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.SecureTempFileCreation) Reported-by: Jonathan Leitschuh <[email protected]> Bug-tracker: JLLeitschuh/security-research#18 Co-authored-by: Moderne <[email protected]>
apache · Mar 5, 2023 · 38da9fa · 38da9fa
1 parent f8f2633
commit 38da9fa
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 35 deletions.
diff --git a/src/main/java/org/apache/maven/shared/io/download/DefaultDownloadManager.java b/src/main/java/org/apache/maven/shared/io/download/DefaultDownloadManager.java
@@ -23,6 +23,7 @@
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.Files;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -121,7 +122,7 @@ public File download( String url, List<TransferListener> transferListeners, Mess
         try
         {
             // create the landing file in /tmp for the downloaded source archive
-            downloaded = File.createTempFile( "download-", null );
+            downloaded = Files.createTempFile( "download-", null ).toFile();
 
             // delete when the JVM exits, to avoid polluting the temp dir...
             downloaded.deleteOnExit();

diff --git a/src/main/java/org/apache/maven/shared/io/location/URLLocation.java b/src/main/java/org/apache/maven/shared/io/location/URLLocation.java
@@ -22,6 +22,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
+import java.nio.file.Files;
 
 import org.apache.maven.shared.utils.io.FileUtils;
 
@@ -66,7 +67,7 @@ protected void initFile()
         // TODO: Log this in the debug log-level...
         if ( unsafeGetFile() == null )
         {
-            File tempFile = File.createTempFile( tempFilePrefix, tempFileSuffix );
+            File tempFile = Files.createTempFile( tempFilePrefix, tempFileSuffix ).toFile();
 
             if ( tempFileDeleteOnExit )
             {

diff --git a/src/test/java/org/apache/maven/shared/io/download/DefaultDownloadManagerTest.java b/src/test/java/org/apache/maven/shared/io/download/DefaultDownloadManagerTest.java
@@ -29,6 +29,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.Collections;
 
 import org.apache.commons.lang3.exception.ExceptionUtils;
@@ -108,7 +109,7 @@ public void testShouldFailToDownloadMalformedURL()
     public void testShouldDownloadFromTempFileWithNoTransferListeners()
         throws IOException, DownloadFailedException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupDefaultMockConfiguration();
@@ -125,7 +126,7 @@ public void testShouldDownloadFromTempFileWithNoTransferListeners()
     public void testShouldDownloadFromTempFileTwiceAndUseCache()
         throws IOException, DownloadFailedException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupDefaultMockConfiguration();
@@ -150,7 +151,7 @@ public void testShouldDownloadFromTempFileTwiceAndUseCache()
     public void testShouldDownloadFromTempFileWithOneTransferListener()
         throws IOException, DownloadFailedException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupDefaultMockConfiguration();
@@ -174,7 +175,7 @@ public void testShouldDownloadFromTempFileWithOneTransferListener()
     public void testShouldFailToDownloadWhenWagonProtocolNotFound()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonManagerGetException( new UnsupportedProtocolException( "not supported" ) );
@@ -200,7 +201,7 @@ public void testShouldFailToDownloadWhenWagonProtocolNotFound()
     public void testShouldFailToDownloadWhenWagonConnectThrowsConnectionException()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonConnectionException( new ConnectionException( "connect error" ) );
@@ -226,7 +227,7 @@ public void testShouldFailToDownloadWhenWagonConnectThrowsConnectionException()
     public void testShouldFailToDownloadWhenWagonConnectThrowsAuthenticationException()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonConnectionException( new AuthenticationException( "bad credentials" ) );
@@ -252,7 +253,7 @@ public void testShouldFailToDownloadWhenWagonConnectThrowsAuthenticationExceptio
     public void testShouldFailToDownloadWhenWagonGetThrowsTransferFailedException()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonGetException( new TransferFailedException( "bad transfer" ) );
@@ -278,7 +279,7 @@ public void testShouldFailToDownloadWhenWagonGetThrowsTransferFailedException()
     public void testShouldFailToDownloadWhenWagonGetThrowsResourceDoesNotExistException()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonGetException( new ResourceDoesNotExistException( "bad resource" ) );
@@ -304,7 +305,7 @@ public void testShouldFailToDownloadWhenWagonGetThrowsResourceDoesNotExistExcept
     public void testShouldFailToDownloadWhenWagonGetThrowsAuthorizationException()
         throws IOException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonGetException( new AuthorizationException( "bad transfer" ) );
@@ -330,7 +331,7 @@ public void testShouldFailToDownloadWhenWagonGetThrowsAuthorizationException()
     public void testShouldFailToDownloadWhenWagonDisconnectThrowsConnectionException()
         throws IOException, DownloadFailedException
     {
-        File tempFile = File.createTempFile( "download-source", "test" );
+        File tempFile = Files.createTempFile( "download-source", "test" ).toFile();
         tempFile.deleteOnExit();
 
         setupMocksWithWagonDisconnectException( new ConnectionException( "not connected" ) );

diff --git a/src/test/java/org/apache/maven/shared/io/location/ArtifactLocationTest.java b/src/test/java/org/apache/maven/shared/io/location/ArtifactLocationTest.java
@@ -21,6 +21,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 
 import junit.framework.TestCase;
 
@@ -37,7 +38,7 @@ public class ArtifactLocationTest
     public void testShouldConstructFromTempFileSpecification()
         throws IOException
     {
-        File f = File.createTempFile( "artifact-location.", ".test" );
+        File f = Files.createTempFile( "artifact-location.", ".test" ).toFile();
         f.deleteOnExit();
 
         Artifact a = new DefaultArtifact( "group", "artifact", VersionRange.createFromVersion( "1" ), null, "jar",
@@ -53,7 +54,7 @@ public void testShouldConstructFromTempFileSpecification()
     public void testShouldRead()
         throws IOException
     {
-        File f = File.createTempFile( "url-location.", ".test" );
+        File f = Files.createTempFile( "url-location.", ".test" ).toFile();
         f.deleteOnExit();
 
         String testStr = "This is a test";

diff --git a/src/test/java/org/apache/maven/shared/io/location/ArtifactLocatorStrategyTest.java b/src/test/java/org/apache/maven/shared/io/location/ArtifactLocatorStrategyTest.java
@@ -21,6 +21,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.Collections;
 
 import junit.framework.TestCase;
@@ -106,7 +107,7 @@ public void testShouldFailToResolveSpecWithTwoTokens()
     public void testShouldResolveSpecWithThreeTokensUsingDefaultType()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );
@@ -150,7 +151,7 @@ public void testShouldResolveSpecWithThreeTokensUsingDefaultType()
     public void testShouldResolveSpecWithThreeTokensUsingCustomizedDefaultType()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );
@@ -194,7 +195,7 @@ public void testShouldResolveSpecWithThreeTokensUsingCustomizedDefaultType()
     public void testShouldResolveSpecWithFourTokens()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );
@@ -238,7 +239,7 @@ public void testShouldResolveSpecWithFourTokens()
     public void testShouldResolveSpecWithFiveTokens()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );
@@ -283,7 +284,7 @@ public void testShouldResolveSpecWithFiveTokens()
     public void testShouldResolveSpecWithFiveTokensAndEmptyTypeToken()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );
@@ -328,7 +329,7 @@ public void testShouldResolveSpecWithFiveTokensAndEmptyTypeToken()
     public void testShouldResolveSpecWithMoreThanFiveTokens()
         throws IOException
     {
-        File tempFile = File.createTempFile( "artifact-location.", ".temp" );
+        File tempFile = Files.createTempFile( "artifact-location.", ".temp" ).toFile();
         tempFile.deleteOnExit();
 
         Artifact artifact = createMock( Artifact.class );

diff --git a/src/test/java/org/apache/maven/shared/io/location/FileLocationTest.java b/src/test/java/org/apache/maven/shared/io/location/FileLocationTest.java
@@ -28,6 +28,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
+import java.nio.file.Files;
 
 import junit.framework.TestCase;
 
@@ -37,7 +38,7 @@ public class FileLocationTest
 
     public void testShouldConstructWithFileThenRetrieveSameFile() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         FileLocation location = new FileLocation( file, file.getAbsolutePath() );
@@ -48,7 +49,7 @@ public void testShouldConstructWithFileThenRetrieveSameFile() throws IOException
 
     public void testShouldReadFileContentsUsingByteBuffer() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         String testStr = "This is a test";
@@ -67,7 +68,7 @@ public void testShouldReadFileContentsUsingByteBuffer() throws IOException
 
     public void testShouldReadFileContentsUsingStream() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         String testStr = "This is a test";
@@ -88,7 +89,7 @@ public void testShouldReadFileContentsUsingStream() throws IOException
 
     public void testShouldReadFileContentsUsingByteArray() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         String testStr = "This is a test";
@@ -107,7 +108,7 @@ public void testShouldReadFileContentsUsingByteArray() throws IOException
 
     public void testShouldReadThenClose() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         String testStr = "This is a test";
@@ -128,7 +129,7 @@ public void testShouldReadThenClose() throws IOException
 
     public void testShouldOpenThenFailToSetFile() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         TestFileLocation location = new TestFileLocation( file.getAbsolutePath() );
@@ -148,7 +149,7 @@ public void testShouldOpenThenFailToSetFile() throws IOException
 
     public void testShouldConstructWithoutFileThenSetFileThenOpen() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         TestFileLocation location = new TestFileLocation( file.getAbsolutePath() );
@@ -159,7 +160,7 @@ public void testShouldConstructWithoutFileThenSetFileThenOpen() throws IOExcepti
 
     public void testShouldConstructWithLocationThenRetrieveEquivalentFile() throws IOException
     {
-        File file = File.createTempFile( "test.", ".file-location" );
+        File file = Files.createTempFile( "test.", ".file-location" ).toFile();
         file.deleteOnExit();
 
         Location location = new TestFileLocation( file.getAbsolutePath() );

diff --git a/src/test/java/org/apache/maven/shared/io/location/FileLocatorStrategyTest.java b/src/test/java/org/apache/maven/shared/io/location/FileLocatorStrategyTest.java
@@ -21,6 +21,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 
 import org.apache.maven.shared.io.logging.DefaultMessageHolder;
 import org.apache.maven.shared.io.logging.MessageHolder;
@@ -33,7 +34,7 @@ public class FileLocatorStrategyTest
 
     public void testShouldResolveExistingTempFileLocation() throws IOException
     {
-        File f = File.createTempFile( "file-locator.", ".test" );
+        File f = Files.createTempFile( "file-locator.", ".test" ).toFile();
         f.deleteOnExit();
 
         FileLocatorStrategy fls = new FileLocatorStrategy();
@@ -51,7 +52,7 @@ public void testShouldResolveExistingTempFileLocation() throws IOException
 
     public void testShouldFailToResolveNonExistentFileLocation() throws IOException
     {
-        File f = File.createTempFile( "file-locator.", ".test" );
+        File f = Files.createTempFile( "file-locator.", ".test" ).toFile();
         f.delete();
 
         FileLocatorStrategy fls = new FileLocatorStrategy();

diff --git a/src/test/java/org/apache/maven/shared/io/location/URLLocationTest.java b/src/test/java/org/apache/maven/shared/io/location/URLLocationTest.java
@@ -22,6 +22,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
+import java.nio.file.Files;
 
 import org.apache.commons.io.FileUtils;
 
@@ -33,7 +34,7 @@ public class URLLocationTest
 
     public void testShouldConstructFromUrlAndTempFileSpecifications() throws IOException
     {
-        File f = File.createTempFile( "url-location.", ".test" );
+        File f = Files.createTempFile( "url-location.", ".test" ).toFile();
         f.deleteOnExit();
 
         URL url = f.toURL();
@@ -43,7 +44,7 @@ public void testShouldConstructFromUrlAndTempFileSpecifications() throws IOExcep
 
     public void testShouldTransferFromTempFile() throws IOException
     {
-        File f = File.createTempFile( "url-location.", ".test" );
+        File f = Files.createTempFile( "url-location.", ".test" ).toFile();
         f.deleteOnExit();
 
         URL url = f.toURL();
@@ -56,7 +57,7 @@ public void testShouldTransferFromTempFile() throws IOException
 
     public void testShouldTransferFromTempFileThenRead() throws IOException
     {
-        File f = File.createTempFile( "url-location.", ".test" );
+        File f = Files.createTempFile( "url-location.", ".test" ).toFile();
         f.deleteOnExit();
 
         String testStr = "This is a test";

diff --git a/src/test/java/org/apache/maven/shared/io/location/URLLocatorStrategyTest.java b/src/test/java/org/apache/maven/shared/io/location/URLLocatorStrategyTest.java
@@ -21,6 +21,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 
 import junit.framework.TestCase;
 
@@ -54,7 +55,7 @@ public void testShouldFailToResolveWithMalformedUrl()
 
     public void testShouldResolveUrlForTempFile() throws IOException
     {
-        File tempFile = File.createTempFile( "prefix.", ".suffix" );
+        File tempFile = Files.createTempFile( "prefix.", ".suffix" ).toFile();
         tempFile.deleteOnExit();
 
         String testStr = "This is a test.";

diff --git a/src/test/java/org/apache/maven/shared/io/scan/SimpleResourceInclusionScannerTest.java b/src/test/java/org/apache/maven/shared/io/scan/SimpleResourceInclusionScannerTest.java
@@ -24,6 +24,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -41,7 +42,7 @@ public void testGetIncludedSources() throws IOException, InclusionScanException
         File baseDir = new File("target" );
         baseDir.deleteOnExit();
         baseDir.mkdirs();
-        File f = File.createTempFile( "source1.", ".test", baseDir );
+        File f = Files.createTempFile( baseDir.toPath(), "source1.", ".test" ).toFile();
 
         Set<String> sourceIncludes = new HashSet<>();
         sourceIncludes.add( "**/*" + f.getName() );