[SNOW-1156046] fix toctou vulnerability in EasyLogginConfig

Snowflake.Data.Tests/IntegrationTests/EasyLoggingIT.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Data;
 using System.IO;
 using System.Runtime.InteropServices;
@@ -73,13 +74,9 @@ public void TestFailToEnableEasyLoggingForWrongConfiguration()
         }
 
         [Test]
+        [Platform(Exclude="Win", Reason="skip test on Windows")]
         public void TestFailToEnableEasyLoggingWhenConfigHasWrongPermissions()
         {
-            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-            {
-                Assert.Ignore("skip test on Windows");
-            }
-
             // arrange
             var configFilePath = CreateConfigTempFile(s_workingDirectory, Config("WARN", s_workingDirectory));
             Syscall.chmod(configFilePath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IWGRP);
@@ -91,19 +88,15 @@ public void TestFailToEnableEasyLoggingWhenConfigHasWrongPermissions()
                 var thrown = Assert.Throws<SnowflakeDbException>(() => conn.Open());
 
                 // assert
-                Assert.That(thrown.Message, Does.Contain("Connection string is invalid: Unable to initialize session"));
+                Assert.That(thrown.Message, Does.Contain("Attempting to read a file with too broad permissions assigned"));
                 Assert.IsFalse(EasyLoggerManager.HasEasyLoggingAppender());
             }
         }
 
         [Test]
+        [Platform(Exclude="Win", Reason="skip test on Windows")]
         public void TestFailToEnableEasyLoggingWhenLogDirectoryNotAccessible()
         {
-            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-            {
-                Assert.Ignore("skip test on Windows");
-            }
-
             // arrange
             var configFilePath = CreateConfigTempFile(s_workingDirectory, Config("WARN", "/"));
             using (IDbConnection conn = new SnowflakeDbConnection())

Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs

@@ -39,24 +39,23 @@ public class EasyLoggingConfigFinderTest
         public void Setup()
         {
             t_fileOperations = new Mock<FileOperations>();
-            t_unixOperations = new Mock<UnixOperations>();
             t_environmentOperations = new Mock<EnvironmentOperations>();
-            t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_environmentOperations.Object);
+            t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_environmentOperations.Object);
             MockHomeDirectory();
             MockExecutionDirectory();
         }
-        
+
         [Test]
         public void TestThatTakesFilePathFromTheInput()
         {
             // arrange
             MockFileFromEnvironmentalVariable();
             MockFileOnDriverPath();
             MockFileOnHomePath();
-            
+
             // act
             var filePath = t_finder.FindConfigFilePath(InputConfigFilePath);
-            
+
             // assert
             Assert.AreEqual(InputConfigFilePath, filePath);
             t_fileOperations.VerifyNoOtherCalls();
@@ -71,14 +70,14 @@ public void TestThatTakesFilePathFromEnvironmentVariableIfInputNotPresent(
             MockFileFromEnvironmentalVariable();
             MockFileOnDriverPath();
             MockFileOnHomePath();
-            
+
             // act
             var filePath = t_finder.FindConfigFilePath(inputFilePath);
-            
+
             // assert
             Assert.AreEqual(EnvironmentalConfigFilePath, filePath);
         }
-        
+
         [Test]
         public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnvironmentVariable()
         {
@@ -88,20 +87,20 @@ public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnviro
 
             // act
             var filePath = t_finder.FindConfigFilePath(null);
-            
+
             // assert
             Assert.AreEqual(s_driverConfigFilePath, filePath);
         }
-        
+
         [Test]
         public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarNorDriverLocation()
         {
             // arrange
             MockFileOnHomePath();
-            
+
             // act
             var filePath = t_finder.FindConfigFilePath(null);
-            
+
             // assert
             Assert.AreEqual(s_homeConfigFilePath, filePath);
         }
@@ -119,32 +118,12 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible()
             Assert.AreEqual(s_homeConfigFilePath, filePath);
         }
 
-        [Test]
-        public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile()
-        {
-            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-            {
-                Assert.Ignore("skip test on Windows");
-            }
-
-            // arrange
-            MockFileOnHomePath();
-            MockHasFlagReturnsTrue();
-
-            // act
-            var thrown = Assert.Throws<Exception>(() => t_finder.FindConfigFilePath(null));
-
-            // assert
-            Assert.IsNotNull(thrown);
-            Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}");
-        }
-
         [Test]
         public void TestThatReturnsNullIfNoWayOfGettingTheFile()
         {
             // act
             var filePath = t_finder.FindConfigFilePath(null);
-            
+
             // assert
             Assert.IsNull(filePath);
         }
@@ -157,7 +136,7 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails()
 
             // act
             var filePath = t_finder.FindConfigFilePath(null);
-            
+
             // assert
             Assert.IsNull(filePath);
             t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once);
@@ -186,7 +165,7 @@ public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist()
 
             // act
             var filePath = t_finder.FindConfigFilePath(null);
-            
+
             // assert
             Assert.IsNull(filePath);
             t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once);
@@ -220,7 +199,7 @@ private static void MockExecutionDirectory()
                 .Setup(e => e.GetExecutionDirectory())
                 .Returns(DriverDirectory);
         }
-        
+
         private static void MockFileOnHomePathDoesNotExist()
         {
             t_fileOperations

Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigParserTest.cs

@@ -33,7 +33,7 @@ public static void AfterAll()
         {
             Directory.Delete(s_workingDirectory, true);
         }
-        
+
         [Test]
         public void TestThatParsesConfigFile()
         {
@@ -59,12 +59,12 @@ public void TestThatParsesConfigFileWithNullValues(string filePath)
 
             // act
             var config = parser.Parse(filePath);
-            
+
             // assert
             Assert.IsNotNull(config);
             Assert.IsNotNull(config.CommonProps);
             Assert.IsNull(config.CommonProps.LogLevel);
-            Assert.IsNull(config.CommonProps.LogPath);            
+            Assert.IsNull(config.CommonProps.LogPath);
         }
 
         [Test]
@@ -77,23 +77,23 @@ public void TestThatReturnsNullWhenNothingToParse(string noFilePath)
 
             // act
             var config = parser.Parse(noFilePath);
-            
+
             // assert
             Assert.IsNull(config);
         }
-        
+
         [Test]
         public void TestThatFailsWhenTheFileDoesNotExist()
         {
             // arrange
             var parser = new EasyLoggingConfigParser();
-            
+
             // act
-            var thrown = Assert.Throws<Exception>(() => parser.Parse(NotExistingFilePath));
-            
+            var thrown = Assert.Throws<FileNotFoundException>(() => parser.Parse(NotExistingFilePath));
+
             // assert
             Assert.IsNotNull(thrown);
-            Assert.AreEqual("Finding easy logging configuration failed", thrown.Message);
+            Assert.AreEqual("No such file or directory", thrown.Message);
         }
 
         [Test, TestCaseSource(nameof(WrongConfigFiles))]

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

@@ -19,15 +19,13 @@ internal class EasyLoggingConfigFinder
         internal const string ClientConfigEnvironmentName = "SF_CLIENT_CONFIG_FILE";
 
         private readonly FileOperations _fileOperations;
-        private readonly UnixOperations _unixOperations;
         private readonly EnvironmentOperations _environmentOperations;
 
-        public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance);
+        public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, EnvironmentOperations.Instance);
 
-        internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, EnvironmentOperations environmentOperations)
+        internal EasyLoggingConfigFinder(FileOperations fileOperations, EnvironmentOperations environmentOperations)
         {
             _fileOperations = fileOperations;
-            _unixOperations = unixFileOperations;
             _environmentOperations = environmentOperations;
         }
 
@@ -38,16 +36,12 @@ internal EasyLoggingConfigFinder()
         public virtual string FindConfigFilePath(string configFilePathFromConnectionString)
         {
             var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string")
-                              ?? GetFilePathEnvironmentVariable()
-                              ?? GetFilePathFromDriverLocation()
-                              ?? GetFilePathFromHomeDirectory();
-            if (configFilePath != null)
-            {
-                CheckIfValidPermissions(configFilePath);
-            }
+                                 ?? GetFilePathEnvironmentVariable()
+                                 ?? GetFilePathFromDriverLocation()
+                                 ?? GetFilePathFromHomeDirectory();
             return configFilePath;
         }
-        
+
         private string GetFilePathEnvironmentVariable()
         {
             var filePath = _environmentOperations.GetEnvironmentVariable(ClientConfigEnvironmentName);
@@ -100,19 +94,5 @@ private string OnlyIfFileExists(string filePath, string directoryDescription)
             }
             return null;
         }
-
-        private void CheckIfValidPermissions(string filePath)
-        {
-            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-                return;
-
-            // Check if others have permissions to modify the file and fail if so
-            if (_unixOperations.CheckFileHasAnyOfPermissions(filePath, FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite))
-            {
-                var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}";
-                s_logger.Error(errorMessage);
-                throw new Exception(errorMessage);
-            }
-        }
     }
 }

Snowflake.Data/Configuration/EasyLoggingConfigParser.cs

@@ -7,8 +7,17 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Runtime.InteropServices;
+using System.Security;
+using System.Text;
+using Microsoft.IdentityModel.Tokens;
+using Mono.Unix;
+using Mono.Unix.Native;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
+using Snowflake.Data.Client;
+using Snowflake.Data.Core;
+using Snowflake.Data.Core.Tools;
 using Snowflake.Data.Log;
 
 namespace Snowflake.Data.Configuration
@@ -22,30 +31,68 @@
         public virtual ClientConfig Parse(string filePath)
         {
             var configFile = TryToReadFile(filePath);
-            return configFile == null ? null : TryToParseFile(configFile);
+            return configFile.IsNullOrEmpty() ? null : TryToParseFile(configFile);
         }
 
+        /// <summary>
+        /// ReadAllText function reads contents of a file at the <paramref name="filePath"/> making sure, in a way not prone to race-conditions, that:
+        ///  - A file is owned by the same user as effective user of the current process.
+        ///  - A file is owned by the same group as effective group of the current process.
+        ///  - A file permissions do not include `forbiddenPermissions` (any others' permissions by default)
+        ///
+        /// </summary>
+        /// <param name="filePath">The file path of the configuration file</param>
+        /// <returns></returns>
+        /// <exception cref="SecurityException">An exception will be thrown if the file is no owned by the user or group</exception>
         private string TryToReadFile(string filePath)
         {
             if (string.IsNullOrEmpty(filePath))
             {
-                return null;
+                return String.Empty;
             }
+
             try
             {
-                return File.ReadAllText(filePath);
+                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                {
+                    var streamReader = new StreamReader(filePath, Encoding.Default);
+                    return streamReader.ReadToEnd();
+                }
+                else
+                {
+                    var handle = VerifyUnixPermissions(filePath);
+
+                    var streamReader = new StreamReader(handle, Encoding.Default);
+                    return streamReader.ReadToEnd();
+                }
             }
             catch (Exception e)
             {
                 var errorMessage = "Finding easy logging configuration failed";
                 s_logger.Error(errorMessage, e);
-                throw new Exception(errorMessage);
+                throw;
             }
         }
 
+        private static UnixStream VerifyUnixPermissions(string filePath)
+        {
+            FileAccessPermissions forbiddenPermissions = FileAccessPermissions.OtherWrite | FileAccessPermissions.GroupWrite;
+            var fileInfo = new UnixFileInfo(path: filePath);
+
+            var handle = fileInfo.OpenRead();
+            if (handle.OwnerUser.UserId != Syscall.geteuid())
+                throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file not owned by the effective user of the current process");
+            if (handle.OwnerGroup.GroupId != Syscall.getegid())
+                throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file not owned by the effective group of the current process");
+            if ((handle.FileAccessPermissions & forbiddenPermissions) != 0)
+                throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file with too broad permissions assigned");
+            return handle;
+        }
+
         private ClientConfig TryToParseFile(string fileContent)
         {
-            try {
+            try
+            {
                 var config = JsonConvert.DeserializeObject<ClientConfig>(fileContent);
                 Validate(config);
                 CheckForUnknownFields(fileContent);

Snowflake.Data/Snowflake.Data.csproj

@@ -13,7 +13,7 @@
     <Authors>Snowflake</Authors>
     <Version>4.0.0</Version>
     <DebugType>Full</DebugType>
-    <LangVersion>7.3</LangVersion>
+    <LangVersion>8.0</LangVersion>
   </PropertyGroup>
 
   <ItemGroup>