Fix for #70007, fallback to old implementation if optimized way fails (#70073)

* Fallback to old implementation if optimized way to query process name fails (#70007)

* Add helper tool that reads the parent process name and writes it to the console (#70007)

* Add test

* Revert "Add helper tool that reads the parent process name and writes it to the console (#70007)"

This reverts commit fa3addb0dd6c61b8cc238238e106ad8f3b6da9ae.

* Remove extra newline

Co-authored-by: David Cantú <dacantu@microsoft.com>

diff --git a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
index f8a8675..7d2c35d 100644 (file)
--- a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
+++ b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
@@ -271,5 +271,8 @@ namespace System
                 return s_isWindowsElevated == 1;
             }
         }
+
+        public static bool IsWindowsAndNotElevated
+            => IsWindows && !IsWindowsAndElevated;
     }
 }

diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
index a7045f0..2b94b16 100644 (file)
--- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
+++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
@@ -872,7 +872,6 @@ namespace System.Diagnostics
             {
                 if (_processName == null)
                 {
-                    EnsureState(State.HaveNonExitedId);
                     // If we already have the name via a populated ProcessInfo
                     // then use that one.
                     if (_processInfo?.ProcessName != null)
@@ -881,12 +880,16 @@ namespace System.Diagnostics
                     }
                     else
                     {
-                        // If we don't have a populated ProcessInfo, then get and cache the process name.
+                        // Ensure that the process is not yet exited
+                        EnsureState(State.HaveNonExitedId);
                         _processName = ProcessManager.GetProcessName(_processId, _machineName);
 
+                        // Fallback to slower ProcessInfo implementation if optimized way did not return a
+                        // process name (e.g. in case of missing permissions for Non-Admin users)
                         if (_processName == null)
                         {
-                            throw new InvalidOperationException(SR.NoProcessInfo);
+                            EnsureState(State.HaveProcessInfo);
+                            _processName = _processInfo!.ProcessName;
                         }
                     }
                 }

diff --git a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs
index b2358fb..cf5507a 100644 (file)
--- a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs
+++ b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs
@@ -2,8 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Collections.Generic;
-using System.Text;
+using System.ComponentModel;
+using System.Security.Principal;
 using System.Threading.Tasks;
 using Xunit.Sdk;
 
@@ -47,5 +47,27 @@ namespace System.Diagnostics.Tests
                 p.Dispose();
             }
         }
+
+        public static string? GetProcessUserName(Process p)
+        {
+            try
+            {
+                if (Interop.OpenProcessToken(p.SafeHandle, 0x8u, out var handle))
+                {
+                    if (Interop.ProcessTokenToSid(handle, out var sid))
+                    {
+                        string userName = sid.Translate(typeof(NTAccount)).ToString();
+                        int indexOfDomain = userName.IndexOf('\\');
+                        if (indexOfDomain != -1)
+                            userName = userName.Substring(indexOfDomain + 1);
+
+                        return userName;
+                    }
+                }
+            }
+            catch (Win32Exception) { } // Process.SafeHandle can throw unauthorized since it uses OpenProcess with PROCESS_ALL_ACCESS.
+
+            return null;
+        }
     }
 }

diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs
index 8d9fb13..cb80e17 100644 (file)
--- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs
+++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs
@@ -504,22 +504,11 @@ namespace System.Diagnostics.Tests
                     throw new SkipTestException($"{p.StartInfo.FileName} has been locked by some other process");
                 }
 
-                if (Interop.OpenProcessToken(p.SafeHandle, 0x8u, out handle))
-                {
-                    SecurityIdentifier sid;
-                    if (Interop.ProcessTokenToSid(handle, out sid))
-                    {
-                        string actualUserName = sid.Translate(typeof(NTAccount)).ToString();
-                        int indexOfDomain = actualUserName.IndexOf('\\');
-                        if (indexOfDomain != -1)
-                            actualUserName = actualUserName.Substring(indexOfDomain + 1);
 
-                        bool isProfileLoaded = GetNamesOfUserProfiles().Any(profile => profile.Equals(username));
+                Assert.Equal(username, Helpers.GetProcessUserName(p));
+                bool isProfileLoaded = GetNamesOfUserProfiles().Any(profile => profile.Equals(username));
+                Assert.True(isProfileLoaded);
 
-                        Assert.Equal(username, actualUserName);
-                        Assert.True(isProfileLoaded);
-                    }
-                }
             }
             finally
             {

diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
index db11065..a13d097 100644 (file)
--- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
+++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
@@ -2545,6 +2545,37 @@ namespace System.Diagnostics.Tests
             }
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindowsAndNotElevated))]
+        public void NonElevatedUser_QueryProcessNameOfSystemProcess()
+        {
+            const string Services = "services";
+
+            string currentProcessUser = Helpers.GetProcessUserName(Process.GetCurrentProcess());
+            Assert.NotNull(currentProcessUser);
+
+            Process? systemOwnedServices = null;
+
+            foreach (var p in Process.GetProcessesByName(Services))
+            {
+                // returns the username of the owner of the process or null if the username can't be queried.
+                // for services.exe, this will be null.
+                string? servicesUser = Helpers.GetProcessUserName(p); 
+
+                // this isn't really verifying that services.exe is owned by SYSTEM, but we are sure it is not owned by the current user.
+                if (servicesUser != currentProcessUser)
+                {
+                    systemOwnedServices = p;
+                    break;
+                }
+            }
+
+            Assert.NotNull(systemOwnedServices);
+            Assert.Equal(Services, systemOwnedServices.ProcessName);
+
+            systemOwnedServices = Process.GetProcessById(systemOwnedServices.Id);
+            Assert.Equal(Services, systemOwnedServices.ProcessName);
+        }
+
         private IReadOnlyList<Process> CreateProcessTree()
         {
             (Process Value, string Message) rootResult = ListenForAnonymousPipeMessage(rootPipeHandleString =>