Fix two issues in Marshal.SecureStringToBSTR (#18851)

1. It wasn't synchronized, even though other operations on SecureString are in order to avoid concurrency problems leading to memory corruption issues (and it's synchronized on netfx).

2. It wasn't throwing an ObjectDisposedException if the instance was disposed.

diff --git a/src/System.Private.CoreLib/shared/System/Security/SecureString.Unix.cs b/src/System.Private.CoreLib/shared/System/Security/SecureString.Unix.cs
index cfeebc1daf99958c322959f081cc1565d5b5af09..ad14bcd296ecd9486eee3295d6e782ad837fca0b 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Security/SecureString.Unix.cs
+++ b/src/System.Private.CoreLib/shared/System/Security/SecureString.Unix.cs
@@ -70,14 +70,6 @@ namespace System.Security
             }
         }
 
-        private void EnsureNotDisposed()
-        {
-            if (_buffer == null)
-            {
-                throw new ObjectDisposedException(GetType().Name);
-            }
-        }
-
         private void ClearCore()
         {
             _decryptedLength = 0;
@@ -143,7 +135,7 @@ namespace System.Security
             _buffer.Write((ulong)(index * sizeof(char)), c);
         }
 
-        internal unsafe IntPtr MarshalToBSTR()
+        internal unsafe IntPtr MarshalToBSTRCore()
         {
             int length = _decryptedLength;
             IntPtr ptr = IntPtr.Zero;

diff --git a/src/System.Private.CoreLib/shared/System/Security/SecureString.Windows.cs b/src/System.Private.CoreLib/shared/System/Security/SecureString.Windows.cs
index 2a800819125378c4b7f350698110b8f292297127..a78fbc222c59e72fc9652251966da9de80f7f6af 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Security/SecureString.Windows.cs
+++ b/src/System.Private.CoreLib/shared/System/Security/SecureString.Windows.cs
@@ -6,7 +6,6 @@ using System.Diagnostics;
 using System.Runtime;
 using System.Runtime.InteropServices;
 using System.Security.Cryptography;
-using Microsoft.Win32;
 
 namespace System.Security
 {
@@ -145,7 +144,7 @@ namespace System.Security
             }
         }
 
-        internal unsafe IntPtr MarshalToBSTR()
+        internal unsafe IntPtr MarshalToBSTRCore()
         {
             int length = _decryptedLength;
             IntPtr ptr = IntPtr.Zero;
@@ -232,14 +231,6 @@ namespace System.Security
             return result;
         }
 
-        private void EnsureNotDisposed()
-        {
-            if (_buffer == null)
-            {
-                throw new ObjectDisposedException(GetType().Name);
-            }
-        }
-
         // -----------------------------
         // ---- PAL layer ends here ----
         // -----------------------------

diff --git a/src/System.Private.CoreLib/shared/System/Security/SecureString.cs b/src/System.Private.CoreLib/shared/System/Security/SecureString.cs
index 5eb66290e38bfa313ed12eb5bebdf7c7afab952f..cdee9c907677c95f68f0f4d7645d294ccaa9bc32 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Security/SecureString.cs
+++ b/src/System.Private.CoreLib/shared/System/Security/SecureString.cs
@@ -156,6 +156,23 @@ namespace System.Security
             }
         }
 
+        private void EnsureNotDisposed()
+        {
+            if (_buffer == null)
+            {
+                throw new ObjectDisposedException(GetType().Name);
+            }
+        }
+
+        internal IntPtr MarshalToBSTR()
+        {
+            lock (_methodLock)
+            {
+                EnsureNotDisposed();
+                return MarshalToBSTRCore();
+            }
+        }
+
         internal unsafe IntPtr MarshalToString(bool globalAlloc, bool unicode)
         {
             lock (_methodLock)