Set Password to SecureString for Wallet (#3468)

* Set `Password` to `SecureString` for Wallet information

* Added `UnitTest` for `SecureStringExtensions`

* Update tests/Neo.Extensions.Tests/UT_SecureStringExtensions.cs

Co-authored-by: Shargon <shargon@gmail.com>

* Update src/Neo.Extensions/SecureStringExtensions.cs

---------

Co-authored-by: Shargon <shargon@gmail.com>

src/Neo.Extensions/Neo.Extensions.csproj

@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.1;net8.0</TargetFrameworks>
     <Nullable>enable</Nullable>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <PackageId>Neo.Extensions</PackageId>
     <PackageTags>NEO;Blockchain;Extensions</PackageTags>
     <OutputPath>../../bin/$(PackageId)</OutputPath>

src/Neo.Extensions/SecureStringExtensions.cs

@@ -0,0 +1,53 @@
+// Copyright (C) 2015-2024 The Neo Project.
+//
+// SecureStringExtensions.cs file belongs to the neo project and is free
+// software distributed under the MIT software license, see the
+// accompanying file LICENSE in the main directory of the
+// repository or http://www.opensource.org/licenses/mit-license.php
+// for more details.
+//
+// Redistribution and use in source and binary forms with or without
+// modifications are permitted.
+
+using System;
+using System.Runtime.InteropServices;
+using System.Security;
+
+namespace Neo.Extensions
+{
+    public static class SecureStringExtensions
+    {
+        public static string? GetClearText(this SecureString secureString)
+        {
+            if (secureString is null)
+                throw new ArgumentNullException(nameof(secureString));
+
+            var unmanagedStringPtr = IntPtr.Zero;
+
+            try
+            {
+                unmanagedStringPtr = Marshal.SecureStringToGlobalAllocUnicode(secureString);
+                return Marshal.PtrToStringUni(unmanagedStringPtr);
+            }
+            finally
+            {
+                Marshal.ZeroFreeGlobalAllocUnicode(unmanagedStringPtr);
+            }
+        }
+
+        public static SecureString ToSecureString(this string value, bool asReadOnly = true)
+        {
+            unsafe
+            {
+                fixed (char* passwordChars = value)
+                {
+                    var securePasswordString = new SecureString(passwordChars, value.Length);
+
+                    if (asReadOnly)
+                        securePasswordString.MakeReadOnly();
+                    return securePasswordString;
+                }
+            }
+        }
+    }
+}

src/Neo/Wallets/NEP6/NEP6Account.cs

@@ -134,7 +134,7 @@ internal void ChangePasswordCommit()
             }
         }
 
-        internal void ChangePasswordRoolback()
+        internal void ChangePasswordRollback()
         {
             nep2KeyNew = null;
         }

src/Neo/Wallets/NEP6/NEP6Wallet.cs

@@ -9,13 +9,15 @@
 // Redistribution and use in source and binary forms with or without
 // modifications are permitted.
 
+using Neo.Extensions;
 using Neo.Json;
 using Neo.SmartContract;
 using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Runtime.InteropServices;
+using System.Security;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
@@ -28,7 +30,7 @@ namespace Neo.Wallets.NEP6
     /// <remarks>https://github.com/neo-project/proposals/blob/master/nep-6.mediawiki</remarks>
     public class NEP6Wallet : Wallet
     {
-        private string password;
+        private SecureString password;
         private string name;
         private Version version;
         private readonly Dictionary<UInt160, NEP6Account> accounts;
@@ -55,10 +57,10 @@ public class NEP6Wallet : Wallet
         /// <param name="name">The name of the wallet. If the wallet is loaded from an existing file, this parameter is ignored.</param>
         public NEP6Wallet(string path, string password, ProtocolSettings settings, string name = null) : base(path, settings)
         {
-            this.password = password;
+            this.password = password.ToSecureString();
             if (File.Exists(path))
             {
-                JObject wallet = (JObject)JToken.Parse(File.ReadAllBytes(path));
+                var wallet = (JObject)JToken.Parse(File.ReadAllBytes(path));
                 LoadFromJson(wallet, out Scrypt, out accounts, out extra);
             }
             else
@@ -80,7 +82,7 @@ public NEP6Wallet(string path, string password, ProtocolSettings settings, strin
         /// <param name="json">The JSON object representing the wallet.</param>
         public NEP6Wallet(string path, string password, ProtocolSettings settings, JObject json) : base(path, settings)
         {
-            this.password = password;
+            this.password = password.ToSecureString();
             LoadFromJson(json, out Scrypt, out accounts, out extra);
         }
 
@@ -91,7 +93,7 @@ private void LoadFromJson(JObject wallet, out ScryptParameters scrypt, out Dicti
             scrypt = ScryptParameters.FromJson((JObject)wallet["scrypt"]);
             accounts = ((JArray)wallet["accounts"]).Select(p => NEP6Account.FromJson((JObject)p, this)).ToDictionary(p => p.ScriptHash);
             extra = wallet["extra"];
-            if (!VerifyPasswordInternal(password))
+            if (!VerifyPasswordInternal(password.GetClearText()))
                 throw new InvalidOperationException("Wrong password.");
         }
 
@@ -144,7 +146,7 @@ public override WalletAccount CreateAccount(byte[] privateKey)
                 ParameterNames = new[] { "signature" },
                 Deployed = false
             };
-            NEP6Account account = new(this, contract.ScriptHash, key, password)
+            NEP6Account account = new(this, contract.ScriptHash, key, password.GetClearText())
             {
                 Contract = contract
             };
@@ -168,7 +170,7 @@ public override WalletAccount CreateAccount(Contract contract, KeyPair key = nul
             if (key == null)
                 account = new NEP6Account(this, nep6contract.ScriptHash);
             else
-                account = new NEP6Account(this, nep6contract.ScriptHash, key, password);
+                account = new NEP6Account(this, nep6contract.ScriptHash, key, password.GetClearText());
             account.Contract = nep6contract;
             AddAccount(account);
             return account;
@@ -188,7 +190,7 @@ public override WalletAccount CreateAccount(UInt160 scriptHash)
         /// <returns>The decrypted private key.</returns>
         internal KeyPair DecryptKey(string nep2key)
         {
-            return new KeyPair(GetPrivateKeyFromNEP2(nep2key, password, ProtocolSettings.AddressVersion, Scrypt.N, Scrypt.R, Scrypt.P));
+            return new KeyPair(GetPrivateKeyFromNEP2(nep2key, password.GetClearText(), ProtocolSettings.AddressVersion, Scrypt.N, Scrypt.R, Scrypt.P));
         }
 
         public override void Delete()
@@ -240,7 +242,7 @@ public override WalletAccount Import(X509Certificate2 cert)
                 ParameterNames = new[] { "signature" },
                 Deployed = false
             };
-            NEP6Account account = new(this, contract.ScriptHash, key, password)
+            NEP6Account account = new(this, contract.ScriptHash, key, password.GetClearText())
             {
                 Contract = contract
             };
@@ -258,7 +260,7 @@ public override WalletAccount Import(string wif)
                 ParameterNames = new[] { "signature" },
                 Deployed = false
             };
-            NEP6Account account = new(this, contract.ScriptHash, key, password)
+            NEP6Account account = new(this, contract.ScriptHash, key, password.GetClearText())
             {
                 Contract = contract
             };
@@ -308,7 +310,7 @@ public override void Save()
 
         public override bool VerifyPassword(string password)
         {
-            return this.password == password;
+            return this.password.GetClearText() == password;
         }
 
         private bool VerifyPasswordInternal(string password)
@@ -358,12 +360,12 @@ public override bool ChangePassword(string oldPassword, string newPassword)
             {
                 foreach (NEP6Account account in accounts.Values)
                     account.ChangePasswordCommit();
-                password = newPassword;
+                password = newPassword.ToSecureString();
             }
             else
             {
                 foreach (NEP6Account account in accounts.Values)
-                    account.ChangePasswordRoolback();
+                    account.ChangePasswordRollback();
             }
             return succeed;
         }

tests/Neo.Extensions.Tests/UT_SecureStringExtensions.cs

@@ -0,0 +1,35 @@
+// Copyright (C) 2015-2024 The Neo Project.
+//
+// UT_SecureStringExtensions.cs file belongs to the neo project and is free
+// software distributed under the MIT software license, see the
+// accompanying file LICENSE in the main directory of the
+// repository or http://www.opensource.org/licenses/mit-license.php
+// for more details.
+//
+// Redistribution and use in source and binary forms with or without
+// modifications are permitted.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Neo.Extensions.Tests
+{
+    [TestClass]
+    public class UT_SecureStringExtensions
+    {
+        [TestMethod]
+        public void Test_String_To_SecureString()
+        {
+            var expected = "Hello World";
+            var expectedSecureString = expected.ToSecureString();
+
+            var actual = expectedSecureString.GetClearText();
+
+            Assert.IsTrue(expectedSecureString.IsReadOnly());
+            Assert.AreEqual(expected, actual);
+        }
+    }
+}

tests/Neo.UnitTests/Wallets/NEP6/UT_NEP6Account.cs

@@ -60,7 +60,7 @@ public void TestChangePassword()
             _account.ChangePasswordPrepare("b", "Satoshi").Should().BeTrue();
             _account.ChangePasswordCommit();
             _account.ChangePasswordPrepare("Satoshi", "b").Should().BeTrue();
-            _account.ChangePasswordRoolback();
+            _account.ChangePasswordRollback();
             _account.VerifyPassword("Satoshi").Should().BeTrue();
         }