From 2d139442bae30c7c0f6e3e09e595f921e223139d Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Thu, 18 Jun 2026 03:42:17 -0400
Subject: [PATCH] =?UTF-8?q?fix(siteruntime):=20CertStoreActor=20=E2=80=94?=
 =?UTF-8?q?=20dispose=20listed=20certs=20+=20reject=20path-traversal=20thu?=
 =?UTF-8?q?mbprints=20(T17)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../Actors/CertStoreActor.cs                  | 32 ++++++++++++++-
 .../Actors/CertStoreActorTests.cs             | 40 +++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/CertStoreActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/CertStoreActor.cs
index 19b60bca..d6e82d97 100644
--- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/CertStoreActor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/CertStoreActor.cs
@@ -64,6 +64,13 @@ public class CertStoreActor : ReceiveActor
 
     private void HandleWrite(WriteCertToLocalStore msg)
     {
+        if (!IsSafeThumbprint(msg.Thumbprint))
+        {
+            _log.Warning("Rejecting write for invalid thumbprint {Thumbprint}", msg.Thumbprint);
+            Sender.Tell(new LocalCertOpAck(false, "invalid thumbprint", null));
+            return;
+        }
+
         try
         {
             Directory.CreateDirectory(_trustedStoreDir);
@@ -82,6 +89,13 @@ public class CertStoreActor : ReceiveActor
 
     private void HandleRemove(RemoveCertFromLocalStore msg)
     {
+        if (!IsSafeThumbprint(msg.Thumbprint))
+        {
+            _log.Warning("Rejecting remove for invalid thumbprint {Thumbprint}", msg.Thumbprint);
+            Sender.Tell(new LocalCertOpAck(false, "invalid thumbprint", null));
+            return;
+        }
+
         try
         {
             var path = Path.Combine(_trustedStoreDir, FileNameFor(msg.Thumbprint));
@@ -133,7 +147,10 @@ public class CertStoreActor : ReceiveActor
             TrustedCertInfo? info = null;
             try
             {
-                var cert = X509CertificateLoader.LoadCertificate(File.ReadAllBytes(file));
+                // The TrustedCertInfo projection copies all needed strings/dates
+                // out before this scope ends, so disposing the cert is safe and
+                // releases the native handle held per loaded certificate.
+                using var cert = X509CertificateLoader.LoadCertificate(File.ReadAllBytes(file));
                 info = new TrustedCertInfo(
                     cert.Thumbprint,
                     cert.Subject,
@@ -156,4 +173,17 @@ public class CertStoreActor : ReceiveActor
     }
 
     private static string FileNameFor(string thumbprint) => $"{thumbprint}.der";
+
+    /// <summary>
+    /// Defense-in-depth path-traversal guard. In production a thumbprint is a
+    /// hex SHA1 (always safe), but <c>RemoveServerCertCommand</c> is CLI-exposed
+    /// and this actor accepts the string unchecked, so a thumbprint such as
+    /// <c>../../etc/foo</c> would otherwise resolve OUTSIDE the trusted-store
+    /// directory once combined into a <c>.der</c> file name. Rejects empty,
+    /// separator-bearing, or dot-dot thumbprints before any filesystem touch.
+    /// </summary>
+    private static bool IsSafeThumbprint(string thumbprint) =>
+        !string.IsNullOrWhiteSpace(thumbprint)
+        && thumbprint.IndexOfAny(Path.GetInvalidFileNameChars()) < 0
+        && !thumbprint.Contains("..", StringComparison.Ordinal);
 }
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/CertStoreActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/CertStoreActorTests.cs
index 216418ce..7b429346 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/CertStoreActorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/CertStoreActorTests.cs
@@ -126,4 +126,44 @@ public class CertStoreActorTests : TestKit, IDisposable
         var ack = ExpectMsg<LocalCertOpAck>();
         ack.Success.Should().BeTrue("removing an absent cert is an idempotent no-op");
     }
+
+    [Fact]
+    public void Write_PathTraversalThumbprint_RejectedWithoutTouchingFilesystem()
+    {
+        var (derBase64, _) = BuildSelfSignedCert();
+        var actor = Sys.ActorOf(Props.Create(() => new CertStoreActor(_options)));
+
+        // A "../escape" thumbprint would otherwise resolve to "<root>/escape.der",
+        // OUTSIDE the trusted-store dir, once combined into a ".der" file name.
+        actor.Tell(new WriteCertToLocalStore(derBase64, "../escape"));
+        var ack = ExpectMsg<LocalCertOpAck>();
+        ack.Success.Should().BeFalse("a path-traversal thumbprint must be rejected");
+        ack.Error.Should().Be("invalid thumbprint");
+
+        // No file escaped above the trusted-store dir (the shared temp root).
+        var escapeRoot = Path.GetDirectoryName(_trustedDir)!;
+        File.Exists(Path.Combine(escapeRoot, "escape.der")).Should().BeFalse(
+            "the actor must not write outside the trusted-store directory");
+    }
+
+    [Fact]
+    public void Remove_PathTraversalThumbprint_RejectedWithoutTouchingFilesystem()
+    {
+        // Seed a decoy file one level up to prove the rejected remove never
+        // reaches the filesystem and therefore cannot delete it.
+        var escapeRoot = Path.GetDirectoryName(_trustedDir)!;
+        Directory.CreateDirectory(escapeRoot);
+        var decoy = Path.Combine(escapeRoot, "escape.der");
+        File.WriteAllBytes(decoy, new byte[] { 0x01, 0x02, 0x03 });
+
+        var actor = Sys.ActorOf(Props.Create(() => new CertStoreActor(_options)));
+
+        actor.Tell(new RemoveCertFromLocalStore("../escape"));
+        var ack = ExpectMsg<LocalCertOpAck>();
+        ack.Success.Should().BeFalse("a path-traversal thumbprint must be rejected");
+        ack.Error.Should().Be("invalid thumbprint");
+
+        File.Exists(decoy).Should().BeTrue(
+            "the actor must not delete files outside the trusted-store directory");
+    }
 }