Fix insecure temporary file creation (#429)

* Fix insecure temporary file creation reported by code analysis
OpenRIAServices · Aug 1, 2023 · 286c839 · 286c839
1 parent a620ae4
commit 286c839
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 6 deletions.
diff --git a/src/OpenRiaServices.Tools/Framework/CreateOpenRiaClientFilesTask.cs b/src/OpenRiaServices.Tools/Framework/CreateOpenRiaClientFilesTask.cs
@@ -785,11 +785,9 @@ private bool GenerateClientProxiesOutOfProcess(string generatedFileName, ClientC
             List<string> arguments = new List<string>();
             SetArgumentListForConsoleApp(arguments, generatedFileName, options, sharedCodeServiceParameters, loggingServer.PipeName);
 
-            // TODO: Fix vulnerability with GetTempFileName, see https://sonarcloud.io/project/issues?resolved=false&severities=BLOCKER%2CCRITICAL%2CMAJOR%2CMINOR&sinceLeakPeriod=true&types=VULNERABILITY&pullRequest=414&id=OpenRIAServices_OpenRiaServices&open=AYi1D8MZVJzuBbc9Xd8Q&tab=why
-            // and add error handling 
-            string filename = Path.GetTempFileName();
-            File.WriteAllLines(filename, arguments);
-            startInfo.Arguments = "@" + filename;
+            string fileName = RiaClientFilesTaskHelpers.CreateAndWriteArgumentsToNewTempFile(arguments);
+
+            startInfo.Arguments = "@" + fileName;
 
             var process = Process.Start(startInfo);
 
@@ -828,7 +826,7 @@ private bool GenerateClientProxiesOutOfProcess(string generatedFileName, ClientC
                 }
                 else
                 {
-                    RiaClientFilesTaskHelpers.SafeFileDelete(filename, this);
+                    RiaClientFilesTaskHelpers.SafeFileDelete(fileName, this);
                 }
                 return success;
             }

diff --git a/src/OpenRiaServices.Tools/Framework/RiaClientFilesTaskHelpers.cs b/src/OpenRiaServices.Tools/Framework/RiaClientFilesTaskHelpers.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
 using System.Linq;
@@ -75,6 +76,47 @@ private static Func<AssemblyLoadContext, AssemblyName, Assembly> GetServerAssemb
     }
 #endif
 
+    /// <summary>
+    /// Create and write arguments to a new file
+    /// </summary>
+    /// <param name="arguments">The arguments that should be written to a file</param>
+    /// <returns>The name of the file created</returns>
+    internal static string CreateAndWriteArgumentsToNewTempFile(IEnumerable<string> arguments)
+    {
+        string fileName = Path.Combine(Path.GetTempPath(), $"openria-codegen-{DateTime.Now:yyyyMMdd-HHmmss-fff}.tmp");
+
+        try
+        {
+            WriteContentsToNewFile(fileName, arguments);
+        }
+        catch (IOException)
+        {
+            // The file could already exist, which would throw an IOExcpetion
+            // Add random file name to the file name and try again
+            fileName += Path.GetRandomFileName();
+            WriteContentsToNewFile(fileName, arguments);
+        }
+
+        return fileName;
+    }
+
+    /// <summary>
+    /// Create a new file with <see cref="FileShare.Read"/> and write the contents to it
+    /// </summary>
+    /// <param name="filePath">The path to the file to be created</param>
+    /// <param name="contents">The contents that should be written to the file</param>
+    internal static void WriteContentsToNewFile(string filePath, IEnumerable<string> contents)
+    {
+        using (var fileStream = new FileStream(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.Read))
+        using (var writer = new StreamWriter(fileStream))
+        {
+            foreach (string line in contents)
+            {
+                writer.WriteLine(line);
+            }
+        }
+    }
+
     /// <summary>
     /// Deletes the specified file in VS-compatible way
     /// </summary>