Process.Unix: consider executable permission while searching PATH.

tmds · tmds · commit bbdc6077cb7c · 2021-07-13T10:53:40.000+02:00
diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroups.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroups.cs
@@ -0,0 +1,50 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class Sys
+    {
+        internal static unsafe uint[]? GetGroups()
+        {
+            const int InitialGroupsLength =
+#if DEBUG
+                1;
+#else
+                64;
+#endif
+            Span<uint> groups = stackalloc uint[InitialGroupsLength];
+            do
+            {
+                int rv;
+                fixed (uint* pGroups = groups)
+                {
+                    rv = Interop.Sys.GetGroups(groups.Length, pGroups);
+                }
+
+                if (rv >= 0)
+                {
+                    // success
+                    return groups.Slice(0, rv).ToArray();
+                }
+                else if (rv == -1 && Interop.Sys.GetLastError() == Interop.Error.EINVAL)
+                {
+                    // increase buffer size
+                    groups = new uint[groups.Length * 2];
+                }
+                else
+                {
+                    // failure
+                    return null;
+                }
+            }
+            while (true);
+        }
+
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGroups", SetLastError = true)]
+        private static extern unsafe int GetGroups(int ngroups, uint* groups);
+    }
+}
diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Permissions.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Permissions.cs
@@ -26,6 +26,8 @@ internal enum Permissions
             S_IROTH = 0x4,
             S_IWOTH = 0x2,
             S_IXOTH = 0x1,
+
+            S_IXUGO = S_IXUSR | S_IXGRP | S_IXOTH,
         }
     }
 }
diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c
@@ -256,6 +256,7 @@ static const Entry s_sysNative[] =
     DllImportEntry(SystemNative_HandleNonCanceledPosixSignal)
     DllImportEntry(SystemNative_SetPosixSignalHandler)
     DllImportEntry(SystemNative_GetPlatformSignalNumber)
+    DllImportEntry(SystemNative_GetGroups)
 };
 
 EXTERN_C const void* SystemResolveDllImport(const char* name);
diff --git a/src/libraries/Native/Unix/System.Native/pal_uid.c b/src/libraries/Native/Unix/System.Native/pal_uid.c
@@ -234,3 +234,11 @@ int32_t SystemNative_GetGroupList(const char* name, uint32_t group, uint32_t* gr
 
     return rv;
 }
+
+int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups)
+{
+    assert(ngroups >= 0);
+    assert(groups != NULL);
+
+    return getgroups(ngroups, groups);
+}
diff --git a/src/libraries/Native/Unix/System.Native/pal_uid.h b/src/libraries/Native/Unix/System.Native/pal_uid.h
@@ -81,3 +81,12 @@ PALEXPORT int32_t SystemNative_GetGroupList(const char* name, uint32_t group, ui
 * Always succeeds.
 */
 PALEXPORT uint32_t SystemNative_GetUid(void);
+
+/**
+* Gets groups associated with current process.
+*
+* Returns number of groups for success.
+* On error, -1 is returned and errno is set.
+* If the buffer is too small, errno is EINVAL.
+*/
+PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups);
diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
@@ -273,6 +273,16 @@
              Link="Common\Interop\Unix\Interop.WaitPid.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Access.cs"
              Link="Common\Interop\Unix\System.Native\Interop.Access.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs"
+             Link="Common\Interop\Unix\Interop.Stat.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs"
+             Link="Common\Interop\Unix\Interop.Permissions.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
+             Link="Common\Interop\Unix\Interop.GetEUid.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs"
+             Link="Common\Interop\Unix\Interop.GetEGid.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetGroups.cs"
+             Link="Common\Interop\Linux\Interop.GetGroups.cs" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsUnix)' == 'true' and '$(IsiOSLike)' != 'true'">
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.ConfigureTerminalForChildProcess.cs"
@@ -332,8 +342,6 @@
              Link="Common\Interop\FreeBSD\Interop.Process.cs" />
     <Compile Include="$(CommonPath)Interop\FreeBSD\Interop.Process.GetProcInfo.cs"
              Link="Common\Interop\FreeBSD\Interop.Process.GetProcInfo.cs" />
-    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs"
-             Link="Common\Unix\System.Native\Interop.Stat.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnknownUnix)' == 'true'">
     <Compile Include="System\Diagnostics\Process.UnknownUnix.cs" />
diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
@@ -17,6 +17,9 @@ namespace System.Diagnostics
     public partial class Process : IDisposable
     {
         private static volatile bool s_initialized;
+        private static uint s_euid;
+        private static uint s_egid;
+        private static uint[]? s_groups;
         private static readonly object s_initializedGate = new object();
         private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim();
 
@@ -743,7 +746,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi)
                 {
                     string subPath = pathParser.ExtractCurrent();
                     path = Path.Combine(subPath, program);
-                    if (File.Exists(path))
+                    if (IsExecutable(path))
                     {
                         return path;
                     }
@@ -752,6 +755,46 @@ private static string[] CreateEnvp(ProcessStartInfo psi)
             return null;
         }
 
+        private static bool IsExecutable(string fullPath)
+        {
+            Interop.Sys.FileStatus fileinfo;
+
+            if (Interop.Sys.Stat(fullPath, out fileinfo) < 0)
+            {
+                return false;
+            }
+
+            // Check if the path is a directory.
+            if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR)
+            {
+                return false;
+            }
+
+            Interop.Sys.Permissions permissions = (Interop.Sys.Permissions)fileinfo.Mode;
+
+            if (s_euid == 0)
+            {
+                // We're root.
+                return (permissions & Interop.Sys.Permissions.S_IXUGO) != 0;
+            }
+
+            if (s_euid == fileinfo.Uid)
+            {
+                // We own the file.
+                return (permissions & Interop.Sys.Permissions.S_IXUSR) != 0;
+            }
+
+            if (s_egid == fileinfo.Gid ||
+                (s_groups != null && Array.BinarySearch(s_groups, fileinfo.Gid) >= 0))
+            {
+                // A group we're a member of owns the file.
+                return (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
+            }
+
+            // Other.
+            return (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;
+        }
+
         private static long s_ticksPerSecond;
 
         /// <summary>Convert a number of "jiffies", or ticks, to a TimeSpan.</summary>
@@ -1021,6 +1064,14 @@ private static unsafe void EnsureInitialized()
                         throw new Win32Exception();
                     }
 
+                    s_euid = Interop.Sys.GetEUid();
+                    s_egid = Interop.Sys.GetEGid();
+                    s_groups = Interop.Sys.GetGroups();
+                    if (s_groups != null)
+                    {
+                        Array.Sort(s_groups);
+                    }
+
                     // Register our callback.
                     Interop.Sys.RegisterForSigChld(&OnSigChild);
                     SetDelayedSigChildConsoleConfigurationHandler();
diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
@@ -185,6 +185,40 @@ public void ProcessNameMatchesScriptName()
             }
         }
 
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void ProcessStart_SkipsNonExecutableFilesOnPATH()
+        {
+            const string ScriptName = "script";
+
+            // Create a directory named ScriptName.
+            string path1 = Path.Combine(TestDirectory, "Path1");
+            Directory.CreateDirectory(Path.Combine(path1, ScriptName));
+
+            // Create a non-executable file named ScriptName
+            string path2 = Path.Combine(TestDirectory, "Path2");
+            Directory.CreateDirectory(path2);
+            File.WriteAllText(Path.Combine(path2, ScriptName), "Not executable");
+
+            // Create an executable script named ScriptName
+            string path3 = Path.Combine(TestDirectory, "Path3");
+            Directory.CreateDirectory(path3);
+            string filename = WriteScriptFile(path3, ScriptName, returnValue: 42);
+
+            // Process.Start ScriptName with the above on PATH.
+            RemoteInvokeOptions options = new RemoteInvokeOptions();
+            options.StartInfo.EnvironmentVariables["PATH"] = $"{path1}:{path2}:{path3}";
+            RemoteExecutor.Invoke(() =>
+            {
+                using (var px = Process.Start(new ProcessStartInfo { FileName = ScriptName }))
+                {
+                    Assert.NotNull(px);
+                    px.WaitForExit();
+                    Assert.True(px.HasExited);
+                    Assert.Equal(42, px.ExitCode);
+                }
+            }, options).Dispose();
+        }
+
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [PlatformSpecific(TestPlatforms.Linux)] // s_allowedProgramsToRun is Linux specific
         public void ProcessStart_UseShellExecute_OnUnix_FallsBackWhenNotRealExecutable()
diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
@@ -1954,7 +1954,7 @@
       <Link>Common\Interop\Unix\System.Native\Interop.GetCwd.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs">
-      <Link>Common\Interop\Unix\System.Native\Interop.GetHostName.cs</Link>
+      <Link>Common\Interop\Unix\System.Native\Interop.GetEGid.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetHostName.cs">
       <Link>Common\Interop\Unix\System.Native\Interop.GetHostName.cs</Link>

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,8 @@ internal enum Permissions`
`26`	`26`	`S_IROTH = 0x4,`
`27`	`27`	`S_IWOTH = 0x2,`
`28`	`28`	`S_IXOTH = 0x1,`
	`29`	`+`
	`30`	`+ S_IXUGO = S_IXUSR \| S_IXGRP \| S_IXOTH,`
`29`	`31`	`}`
`30`	`32`	`}`
`31`	`33`	`}`