diff --git a/src/NSmartProxy.SecurityTests/NSmartProxy.SecurityTests.csproj b/src/NSmartProxy.SecurityTests/NSmartProxy.SecurityTests.csproj new file mode 100644 index 0000000..d01aecf --- /dev/null +++ b/src/NSmartProxy.SecurityTests/NSmartProxy.SecurityTests.csproj @@ -0,0 +1,19 @@ + + + + net8.0 + false + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/NSmartProxy.SecurityTests/SecurityIssuesTests.cs b/src/NSmartProxy.SecurityTests/SecurityIssuesTests.cs new file mode 100644 index 0000000..d1b111e --- /dev/null +++ b/src/NSmartProxy.SecurityTests/SecurityIssuesTests.cs @@ -0,0 +1,124 @@ +using System; +using System.IO; +using System.Security.Cryptography.X509Certificates; +using NSmartProxy.Extension; +using NUnit.Framework; + +namespace NSmartProxy.SecurityTests +{ + [TestFixture] + public class SecurityIssuesTests + { + [Test] + public void CAGen_UsesHardcodedPassword_SecurityVulnerability() + { + // Arrange + var certificateName = "test"; + + // Act + var certificate = CAGen.GenerateCA(certificateName); + + // Assert + // This test demonstrates that the certificate can be accessed with the hardcoded password + var exportedCert = certificate.Export(X509ContentType.Pfx, "WeNeedASaf3rPassword"); + Assert.That(exportedCert, Is.Not.Null); + + // This is a security issue - the password is hardcoded and predictable + Assert.That(exportedCert.Length, Is.GreaterThan(0), "Certificate should be exportable with hardcoded password"); + } + + [Test] + public void CAGen_HardcodedPasswordIsExposed_SecurityIssue() + { + // This test verifies that the hardcoded password is indeed a security risk + var certificateName = "test"; + var certificate = CAGen.GenerateCA(certificateName); + + // The fact that we can export with this specific password proves it's hardcoded + Assert.That(() => + { + var export = certificate.Export(X509ContentType.Pfx, "WeNeedASaf3rPassword"); + }, Throws.Nothing, "Certificate exports with hardcoded password - this is a security vulnerability"); + } + + [Test] + public void CAGen_SupportsCustomPassword_SecurityImprovement() + { + // Test that the new secure password parameter works + var certificateName = "test"; + var customPassword = "MySecurePassword123!"; + + var certificate = CAGen.GenerateCA(certificateName, null, customPassword); + + // Should be able to export with custom password + Assert.That(() => + { + var export = certificate.Export(X509ContentType.Pfx, customPassword); + }, Throws.Nothing, "Certificate should export with custom password"); + + // Should NOT be able to export with old hardcoded password + Assert.That(() => + { + var export = certificate.Export(X509ContentType.Pfx, "WeNeedASaf3rPassword"); + }, Throws.Exception, "Certificate should NOT export with old hardcoded password"); + } + + [Test] + public void CAGen_GeneratesRandomPasswordWhenNoneProvided_SecurityImprovement() + { + // Test that random passwords are generated when none provided + var certificateName = "test"; + + var certificate1 = CAGen.GenerateCA(certificateName); + var certificate2 = CAGen.GenerateCA(certificateName); + + // Both certificates should exist but should not be exportable with hardcoded password + Assert.That(certificate1, Is.Not.Null); + Assert.That(certificate2, Is.Not.Null); + + // Verify they can't be exported with old hardcoded password + Assert.That(() => + { + var export = certificate1.Export(X509ContentType.Pfx, "WeNeedASaf3rPassword"); + }, Throws.Exception, "Certificate should NOT export with old hardcoded password"); + } + + [Test] + public void PathTraversal_FileWriteVulnerability_Demonstration() + { + // This test demonstrates the potential for path traversal attacks + // Note: This is for demonstration purposes - the actual API would need to be called + + var baseLogPath = "./temp"; + var maliciousFileName = "../../../etc/passwd"; // Potential path traversal + var targetPath = baseLogPath + "/" + maliciousFileName; + + // This shows how the current code could be vulnerable to path traversal + Assert.That(targetPath.Contains("../"), Is.True, "Path contains traversal characters - potential security risk"); + } + + [Test] + public void PathSanitization_PreventDirectoryTraversal_SecurityImprovement() + { + // Test that Path.GetFileName properly sanitizes filenames + var maliciousFileName = "../../../etc/passwd"; + var sanitizedFileName = Path.GetFileName(maliciousFileName); + + // Should only contain the filename, not directory traversal + Assert.That(sanitizedFileName, Is.EqualTo("passwd")); + Assert.That(sanitizedFileName.Contains("../"), Is.False, "Sanitized filename should not contain directory traversal"); + } + + [Test] + public void DefaultCredentials_AdminAccountExists_SecurityIssue() + { + // This test documents the existence of default admin credentials + var defaultUsername = "admin"; + var defaultPassword = "admin"; + + // These are the default credentials created in HttpServer_APIs constructor + Assert.That(defaultUsername, Is.EqualTo("admin"), "Default admin username is predictable"); + Assert.That(defaultPassword, Is.EqualTo("admin"), "Default admin password is weak and predictable"); + } + } +} \ No newline at end of file diff --git a/src/NSmartProxy/Extension/CAGen.cs b/src/NSmartProxy/Extension/CAGen.cs index 5254294..29e29da 100644 --- a/src/NSmartProxy/Extension/CAGen.cs +++ b/src/NSmartProxy/Extension/CAGen.cs @@ -4,6 +4,7 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; +using NSmartProxy.Infrastructure; namespace NSmartProxy.Extension { @@ -14,8 +15,9 @@ public class CAGen /// /// /// + /// 证书密码,如果为空则生成随机密码 /// - public static X509Certificate2 GenerateCA(string CertificateName,string hosts = null) + public static X509Certificate2 GenerateCA(string CertificateName, string hosts = null, string password = null) { SubjectAlternativeNameBuilder sanBuilder = new SubjectAlternativeNameBuilder(); sanBuilder.AddIpAddress(IPAddress.Loopback); @@ -52,8 +54,15 @@ public static X509Certificate2 GenerateCA(string CertificateName,string hosts = //certificate.FriendlyName = CertificateName; //return certificate; - return new X509Certificate2(certificate.Export(X509ContentType.Pfx, "WeNeedASaf3rPassword"), - "WeNeedASaf3rPassword", X509KeyStorageFlags.Exportable); + // 使用提供的密码或生成安全的随机密码 + if (string.IsNullOrEmpty(password)) + { + password = RandomHelper.NextString(32, true); // 生成32位强密码 + Server.Logger?.Debug($"Generated secure random password for certificate: {CertificateName}"); + } + + return new X509Certificate2(certificate.Export(X509ContentType.Pfx, password), + password, X509KeyStorageFlags.Exportable); } } diff --git a/src/NSmartProxy/Extension/HttpServer_APIs.cs b/src/NSmartProxy/Extension/HttpServer_APIs.cs index e6060f0..5a4460d 100644 --- a/src/NSmartProxy/Extension/HttpServer_APIs.cs +++ b/src/NSmartProxy/Extension/HttpServer_APIs.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Net; using System.Net.Sockets; +using System.Security; using System.Security.Cryptography.X509Certificates; using System.Text; using NSmartProxy.Authorize; @@ -38,7 +39,13 @@ public HttpServerApis(IServerContext serverContext, IDbOperator dbOperator, stri //如果库中没有任何记录,则增加默认用户 if (Dbop.GetLength() < 1) { - AddUserV2("admin", "admin", "1"); + // 生成随机管理员密码以提高安全性 + var randomPassword = RandomHelper.NextString(16, true); + AddUserV2("admin", randomPassword, "1"); + + // 记录默认凭据到日志(仅记录用户名,不记录密码) + Server.Logger.Info($"创建了默认管理员账户: admin,密码已随机生成。请立即登录并修改密码!"); + Server.Logger.Info($"Default admin account created with random password. Please login and change password immediately!"); } } @@ -348,7 +355,8 @@ public void UpdateUser(string oldUserName, string newUserName, string userPwd, s User user = Dbop.Get(oldUserName)?.ToObject(); user.isAdmin = isAdmin; user.userName = newUserName; - if (userPwd != "XXXXXXXX") + // 使用更安全的密码检查逻辑:只有在密码非空且不是特殊标记时才更新 + if (!string.IsNullOrEmpty(userPwd) && userPwd != "XXXXXXXX" && userPwd.Trim().Length > 0) { user.userPwd = EncryptHelper.SHA256(userPwd); } @@ -795,11 +803,23 @@ private string FormatExtension(X509ExtensionCollection cert2Extensions) public string GenerateCA(string hosts) { var caName = RandomHelper.NextString(10, false); - X509Certificate2 ca = CAGen.GenerateCA(caName, hosts); - var export = ca.Export(X509ContentType.Pfx); + // 生成安全的随机密码 + var certificatePassword = RandomHelper.NextString(32, true); + X509Certificate2 ca = CAGen.GenerateCA(caName, hosts, certificatePassword); + var export = ca.Export(X509ContentType.Pfx, certificatePassword); string baseLogPath = "./temp"; - string fileName = "_" + caName + ".pfx"; - string targetPath = baseLogPath + "/" + fileName; + // 防止路径遍历攻击:仅使用文件名,移除目录路径 + string safeFileName = "_" + Path.GetFileName(caName) + ".pfx"; + string targetPath = Path.Combine(baseLogPath, safeFileName); + + // 确保目标路径在安全目录内 + string fullBasePath = Path.GetFullPath(baseLogPath); + string fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(fullBasePath)) + { + throw new SecurityException("Invalid file path detected - potential directory traversal attack"); + } + DirectoryInfo dir = new DirectoryInfo(baseLogPath); if (!dir.Exists) { @@ -808,7 +828,7 @@ public string GenerateCA(string hosts) // File.Move(fileInfo.FullName, baseLogPath + "/" + port + ".pfx"); File.WriteAllBytes(targetPath, export); - return fileName; + return safeFileName; } [FileUpload] @@ -816,7 +836,18 @@ public string GenerateCA(string hosts) public string UploadTempFile(FileInfo fileInfo) { string baseLogPath = "./temp"; - string targetPath = baseLogPath + "/" + fileInfo.Name; + // 防止路径遍历攻击:仅使用文件名,移除目录路径 + string safeFileName = Path.GetFileName(fileInfo.Name); + string targetPath = Path.Combine(baseLogPath, safeFileName); + + // 确保目标路径在安全目录内 + string fullBasePath = Path.GetFullPath(baseLogPath); + string fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(fullBasePath)) + { + throw new SecurityException("Invalid file path detected - potential directory traversal attack"); + } + DirectoryInfo dir = new DirectoryInfo(baseLogPath); if (!dir.Exists) {