From d48547d4651aeaac98c4fa659d0e8d468b17b2f9 Mon Sep 17 00:00:00 2001
From: Chrison Simtian <csimon@chrison.dev>
Date: Mon, 1 Jun 2026 12:49:03 +1200
Subject: [PATCH] fix(security): stop leaking DSM credentials (login URL +
 record ToString)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two credential-exposure fixes:

1. LoginAsync sent account + password in the auth.cgi *query string* of a
   GET, so they landed in DSM access logs, proxy logs, and HttpClient
   request-URI logging even over TLS (CWE-598). Send the SYNO.API.Auth
   parameters as a POST form body instead.

2. SynologyOptions and SynologySshOptions are records whose synthesized
   ToString() printed Password — leaking it into any log/interpolation.
   Override ToString() to emit `Password = ***`.

Adds a regression test asserting the password never appears in ToString().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/SynoSharp/Ssh/SynologySshOptions.cs | 10 +++++++++
 src/SynoSharp/SynologyApiClient.cs      | 29 ++++++++++++++++++++-----
 src/SynoSharp/SynologyOptions.cs        |  8 +++++++
 tests/SynoSharp.Tests/SynologyTests.cs  | 16 ++++++++++++++
 4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/src/SynoSharp/Ssh/SynologySshOptions.cs b/src/SynoSharp/Ssh/SynologySshOptions.cs
index f0c2dfb..aa69536 100644
--- a/src/SynoSharp/Ssh/SynologySshOptions.cs
+++ b/src/SynoSharp/Ssh/SynologySshOptions.cs
@@ -60,4 +60,14 @@ public sealed record SynologySshOptions
             PrivateKeyPath = Environment.GetEnvironmentVariable("SYNOLOGY_SSH_KEY"),
         };
     }
+
+    /// <summary>
+    /// Redacted representation. The synthesized record <c>ToString()</c> would
+    /// otherwise print <see cref="Password"/> (the login + sudo password),
+    /// leaking it into any log or interpolated string. The password is never emitted.
+    /// </summary>
+    public override string ToString() =>
+        $"SynologySshOptions {{ Host = {Host}, Port = {Port}, Username = {Username}, " +
+        $"Password = {(Password is null ? "null" : "***")}, PrivateKeyPath = {PrivateKeyPath}, " +
+        $"AcceptAnyHostKey = {AcceptAnyHostKey} }}";
 }
diff --git a/src/SynoSharp/SynologyApiClient.cs b/src/SynoSharp/SynologyApiClient.cs
index d6fbe30..244823d 100644
--- a/src/SynoSharp/SynologyApiClient.cs
+++ b/src/SynoSharp/SynologyApiClient.cs
@@ -47,14 +47,33 @@ public SynologyApiClient(SynologyOptions options, HttpClient? httpClient = null)
             : new Uri(options.BaseUrl.AbsoluteUri + "/");
     }
 
-    /// <summary>Authenticate (<c>SYNO.API.Auth</c>) and cache the session id.</summary>
+    /// <summary>
+    /// Authenticate (<c>SYNO.API.Auth</c>) and cache the session id.
+    /// <para>
+    /// Credentials are sent as a <b>POST form body</b>, not in the query string,
+    /// so the account and password never land in DSM access logs, proxy logs, or
+    /// <c>HttpClient</c> request-URI logging. DSM 7.x accepts the auth parameters
+    /// as form fields on <c>auth.cgi</c>.
+    /// </para>
+    /// </summary>
     public async Task LoginAsync(CancellationToken cancellationToken = default)
     {
-        var query = $"webapi/auth.cgi?api=SYNO.API.Auth&version=3&method=login" +
-            $"&account={Uri.EscapeDataString(_options.Username)}" +
-            $"&passwd={Uri.EscapeDataString(_options.Password)}&session=DSM&format=sid";
+        using var form = new FormUrlEncodedContent(new Dictionary<string, string>
+        {
+            ["api"] = "SYNO.API.Auth",
+            ["version"] = "3",
+            ["method"] = "login",
+            ["account"] = _options.Username,
+            ["passwd"] = _options.Password,
+            ["session"] = "DSM",
+            ["format"] = "sid",
+        });
 
-        var envelope = await _http.GetFromJsonAsync<JsonElement>(query, cancellationToken).ConfigureAwait(false);
+        using var response = await _http.PostAsync("webapi/auth.cgi", form, cancellationToken).ConfigureAwait(false);
+        response.EnsureSuccessStatusCode();
+
+        var envelope = await response.Content
+            .ReadFromJsonAsync<JsonElement>(cancellationToken).ConfigureAwait(false);
         if (!envelope.TryGetProperty("success", out var ok) || !ok.GetBoolean())
         {
             throw new InvalidOperationException("Synology login failed (SYNO.API.Auth).");
diff --git a/src/SynoSharp/SynologyOptions.cs b/src/SynoSharp/SynologyOptions.cs
index 7a47704..381e4ff 100644
--- a/src/SynoSharp/SynologyOptions.cs
+++ b/src/SynoSharp/SynologyOptions.cs
@@ -34,4 +34,12 @@ public sealed record SynologyOptions
 
         return new SynologyOptions { BaseUrl = new Uri(baseUrl), Username = user, Password = pass, VerifyTls = verifyTls };
     }
+
+    /// <summary>
+    /// Redacted representation. The synthesized record <c>ToString()</c> would
+    /// otherwise print <see cref="Password"/>, leaking it into any log or
+    /// interpolated string. The password is never emitted.
+    /// </summary>
+    public override string ToString() =>
+        $"SynologyOptions {{ BaseUrl = {BaseUrl}, Username = {Username}, Password = ***, VerifyTls = {VerifyTls} }}";
 }
diff --git a/tests/SynoSharp.Tests/SynologyTests.cs b/tests/SynoSharp.Tests/SynologyTests.cs
index 31e0a7c..5f6c38e 100644
--- a/tests/SynoSharp.Tests/SynologyTests.cs
+++ b/tests/SynoSharp.Tests/SynologyTests.cs
@@ -16,6 +16,22 @@ public void VerifyTls_defaults_to_true()
 
         Assert.True(options.VerifyTls);
     }
+
+    [Fact]
+    public void ToString_does_not_leak_the_password()
+    {
+        var options = new SynologyOptions
+        {
+            BaseUrl = new Uri("https://nas:5001"),
+            Username = "u",
+            Password = "super-secret-password",
+        };
+
+        var text = options.ToString();
+
+        Assert.DoesNotContain("super-secret-password", text);
+        Assert.Contains("***", text);
+    }
 }
 
 /// <summary>