From e8769fd8a886163692641a7fbc1edfe258a5c2bd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 05:08:39 -0400 Subject: [PATCH] =?UTF-8?q?refactor(adminui):=20cert=20manager=20review=20?= =?UTF-8?q?polish=20=E2=80=94=20char.IsAsciiHexDigit,=20filtered=20catch,?= =?UTF-8?q?=20TOCTOU=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Certificates/CertificateStoreManager.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Certificates/CertificateStoreManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Certificates/CertificateStoreManager.cs index fdf6ebc7..faba0946 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Certificates/CertificateStoreManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Certificates/CertificateStoreManager.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Microsoft.Extensions.Configuration; @@ -87,6 +88,9 @@ public sealed class CertificateStoreManager if (File.Exists(dest)) dest = Path.Combine(destDir, $"{Path.GetFileNameWithoutExtension(src)}_{thumbprint}{Path.GetExtension(src)}"); + // Narrow TOCTOU window: if a concurrent admin action created `dest` between the + // check above and here, File.Move throws — we surface that as a Fail (no data loss, + // no overwrite of a different cert). Cert-store edits are rare manual operations. File.Move(src, dest); return CertActionResult.Ok(); } @@ -110,7 +114,10 @@ public sealed class CertificateStoreManager if (string.Equals(cert.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) return file; } - catch { /* ignore unreadable entries */ } + catch (Exception ex) when (ex is CryptographicException or IOException or UnauthorizedAccessException) + { + /* ignore unreadable/corrupt entries — a bad DER must not abort enumeration */ + } } return null; } @@ -118,5 +125,5 @@ public sealed class CertificateStoreManager private static bool IsValidThumbprint(string thumbprint) => !string.IsNullOrEmpty(thumbprint) && (thumbprint.Length == 40 || thumbprint.Length == 64) - && thumbprint.All(Uri.IsHexDigit); + && thumbprint.All(char.IsAsciiHexDigit); }