Fix read/write directory permission checks (#151)

* Fix read/write directory permission checks * Update logic to take specific Deny rules into account which may take precedence * Create abstract base class for file system utils Co-authored-by: Kevin Sigmund <[email protected]>
microsoft · Sep 28, 2022 · b9f85e6 · b9f85e6
1 parent 52ae144
commit b9f85e6
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 93 deletions.
diff --git a/src/Microsoft.Sbom.Api/SBOMGenerator.cs b/src/Microsoft.Sbom.Api/SBOMGenerator.cs
@@ -36,7 +36,7 @@ public SBOMGenerator()
         {
             kernel = new StandardKernel(new Bindings());
             configurationBuilder = new ApiConfigurationBuilder();
-            fileSystemUtils = new FileSystemUtils();
+            fileSystemUtils = new WindowsFileSystemUtils();
         }
 
         public SBOMGenerator(StandardKernel kernel, IFileSystemUtils fileSystemUtils)

diff --git a/src/Microsoft.Sbom.Common/FileSystemUtils.cs b/src/Microsoft.Sbom.Common/FileSystemUtils.cs
@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -12,7 +11,7 @@ namespace Microsoft.Sbom.Common
     /// <summary>
     /// A wrapper class to make the filesystem methods unit testable.
     /// </summary>
-    public class FileSystemUtils : IFileSystemUtils
+    public abstract class FileSystemUtils : IFileSystemUtils
     {
         private readonly EnumerationOptions dontFollowSymlinks = new EnumerationOptions
         {
@@ -60,95 +59,9 @@ public class FileSystemUtils : IFileSystemUtils
             Constants.DefaultStreamBufferSize,
             FileOptions.Asynchronous);
 
-        virtual public bool DirectoryHasReadPermissions(string directoryPath)
-        {
-            try
-            {
-                var readAllow = false;
-                var readDeny = false;
-                var accessControlList = GetDirectorySecurity(directoryPath);
-                if (accessControlList == null)
-                {
-                    return false;
-                }
-
-                //get the access rules that pertain to a valid SID/NTAccount.
-                var accessRules = accessControlList.GetAccessRules(true, true, typeof(System.Security.Principal.SecurityIdentifier));
-                if (accessRules == null)
-                {
-                    return false;
-                }
-
-                //we want to go over these rules to ensure a valid SID has access
-                foreach (FileSystemAccessRule rule in accessRules)
-                {
-                    if ((FileSystemRights.Read & rule.FileSystemRights) != FileSystemRights.Read)
-                    {
-                        continue;
-                    }
-
-                    if (rule.AccessControlType == AccessControlType.Allow)
-                    {
-                        readAllow = true;
-                    }
-                    else if (rule.AccessControlType == AccessControlType.Deny)
-                    {
-                        readDeny = true;
-                    }
-                }
-
-                return readAllow && !readDeny;
-            }
-            catch (Exception)
-            {
-                // TODO Add logger with debug
-                return false;
-            }
-        }
-
-        virtual public bool DirectoryHasWritePermissions(string directoryPath)
-        {
-            try
-            {
-                var writeAllow = false;
-                var writeDeny = false;
-                var accessControlList = GetDirectorySecurity(directoryPath);
-                if (accessControlList == null)
-                {
-                    return false;
-                }
-
-                var accessRules = accessControlList.GetAccessRules(true, true, typeof(System.Security.Principal.SecurityIdentifier));
-                if (accessRules == null)
-                {
-                    return false;
-                }
-
-                foreach (FileSystemAccessRule rule in accessRules)
-                {
-                    if ((FileSystemRights.Write & rule.FileSystemRights) != FileSystemRights.Write)
-                        {
-                        continue;
-                    }
-
-                    if (rule.AccessControlType == AccessControlType.Allow)
-                        {
-                        writeAllow = true;
-                    }
-                    else if (rule.AccessControlType == AccessControlType.Deny)
-                        {
-                        writeDeny = true;
-                    }
-                }
-
-                return writeAllow && !writeDeny;
-            }
-            catch (Exception)
-            {
-                // TODO Add logger with debug
-                return false;
-            }
-        }
+        abstract public bool DirectoryHasReadPermissions(string directoryPath);
+
+        abstract public bool DirectoryHasWritePermissions(string directoryPath);
 
         public DirectoryInfo CreateDirectory(string path) => Directory.CreateDirectory(path);
 

diff --git a/src/Microsoft.Sbom.Common/FileSystemUtilsProvider.cs b/src/Microsoft.Sbom.Common/FileSystemUtilsProvider.cs
@@ -26,7 +26,7 @@ protected override IFileSystemUtils CreateInstance(IContext context)
             bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
             if (isWindows)
             {
-                return new FileSystemUtils();
+                return new WindowsFileSystemUtils();
             }
 
             return new UnixFileSystemUtils();

diff --git a/src/Microsoft.Sbom.Common/WindowsFileSystemUtils.cs b/src/Microsoft.Sbom.Common/WindowsFileSystemUtils.cs
@@ -0,0 +1,46 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.IO;
+using System.Linq;
+using System.Security.AccessControl;
+using System.Security.Principal;
+
+namespace Microsoft.Sbom.Common
+{
+    public class WindowsFileSystemUtils : FileSystemUtils
+    {
+        override public bool DirectoryHasReadPermissions(string directoryPath) => DirectoryHasRights(directoryPath, FileSystemRights.Read);
+
+        override public bool DirectoryHasWritePermissions(string directoryPath) => DirectoryHasRights(directoryPath, FileSystemRights.Write);
+
+        // Get the collection of authorization rules that apply to the directory
+        private bool DirectoryHasRights(string directoryPath, FileSystemRights fileSystemRights)
+        {
+            try
+            {   
+                WindowsIdentity current = WindowsIdentity.GetCurrent();
+                var directoryInfo = new DirectoryInfo(directoryPath);
+
+                return HasAccessControlType(AccessControlType.Allow) && !HasAccessControlType(AccessControlType.Deny);
+
+                // Check if the current user has or does not have the specified rights (either Allow or Deny)
+                bool HasAccessControlType(AccessControlType accessControlType)
+                { 
+                    var accessRules = directoryInfo.GetAccessControl().GetAccessRules(true, true, typeof(SecurityIdentifier))
+                    .Cast<FileSystemAccessRule>()
+                    .Any(rule => (current.Groups.Contains(rule.IdentityReference) || current.User.Equals(rule.IdentityReference))
+                        && ((fileSystemRights & rule.FileSystemRights) == fileSystemRights)
+                        && (rule.AccessControlType == accessControlType));
+                    return accessRules;
+                }
+            }
+            catch (Exception)
+            {
+                // TODO Add logger with debug
+                return false;
+            }
+        }
+    }
+}