Fix stackalloc reuse bug in Environment.UserDomainName (dotnet/coreclr#26994)

* Fix stackalloc reuse bug in Environment.UserDomainName

According to the comment, this code is never expected to be used (so it should either be deleted or the comment fixed), but in the meantime, if it were to be executed, things could go badly due to potentially passing the same buffer into LookupAccountNameW for two different arguments.

* Fix existing ValueStringBuilder usage to dispose the builder

Commit migrated from https://github.com/dotnet/coreclr/commit/814c282db30906a1de496aca4b2654b688be09b5

diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs
index 1ea2461..80153c9 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs
@@ -143,7 +143,9 @@ namespace System
                     name = name.Slice(index + 1);
                 }
 
-                return name.ToString();
+                string result = name.ToString();
+                builder.Dispose();
+                return result;
             }
         }
 
@@ -185,7 +187,8 @@ namespace System
                 if (index != -1)
                 {
                     // In the form of DOMAIN\User, cut off \User and return
-                    return name.Slice(0, index).ToString();
+                    builder.Length = index;
+                    return builder.ToString();
                 }
 
                 // In theory we should never get use out of LookupAccountNameW as the above API should
@@ -194,7 +197,7 @@ namespace System
                 // Domain names aren't typically long.
                 // https://support.microsoft.com/en-us/help/909264/naming-conventions-in-active-directory-for-computers-domains-sites-and
                 Span<char> initialDomainNameBuffer = stackalloc char[64];
-                var domainBuilder = new ValueStringBuilder(initialBuffer);
+                var domainBuilder = new ValueStringBuilder(initialDomainNameBuffer);
                 uint length = (uint)domainBuilder.Capacity;
 
                 // This API will fail to return the domain name without a buffer for the SID.
@@ -216,6 +219,7 @@ namespace System
                     domainBuilder.EnsureCapacity((int)length);
                 }
 
+                builder.Dispose();
                 domainBuilder.Length = (int)length;
                 return domainBuilder.ToString();
             }

diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
index 157e527..e7c6c4d 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
@@ -29,9 +29,14 @@ namespace System
                 builder.Length = (int)length;
 
                 // If we have a tilde in the path, make an attempt to expand 8.3 filenames
-                return builder.AsSpan().Contains('~')
-                    ? PathHelper.TryExpandShortFileName(ref builder, null)
-                    : builder.ToString();
+                if (builder.AsSpan().Contains('~'))
+                {
+                    string result = PathHelper.TryExpandShortFileName(ref builder, null);
+                    builder.Dispose();
+                    return result;
+                }
+
+                return builder.ToString();
             }
             set
             {