From b8427b10cecceeb1d471fa57878a3772619984db Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:08:26 -0400 Subject: [PATCH] fix: XSS, DPAPI scope, and rate-limiter key eviction - PlanShare dashboard: rebuild referrer table with DOM + textContent instead of innerHTML, so legacy rows containing hostile strings can't execute script. - PlanShare /api/event: drop the referrer entirely when Uri.TryCreate rejects it, instead of persisting the raw client-supplied string. - WindowsCredentialService: save credentials with Enterprise scope (current-user, DPAPI-encrypted) instead of LocalMachine (readable by every user on the box). - PlanShare RateLimiter: add Sweep() that prunes and TryRemoves empty keys, invoked from CleanupService's existing hourly tick so the dictionary no longer grows unbounded across unique IPs. Co-Authored-By: Claude Opus 4.7 (1M context) --- server/PlanShare/Program.cs | 57 ++++++++++++++++--- server/PlanShare/dashboard.html | 23 ++++++-- .../Services/WindowsCredentialService.cs | 2 +- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/server/PlanShare/Program.cs b/server/PlanShare/Program.cs index f04964a..097fea3 100644 --- a/server/PlanShare/Program.cs +++ b/server/PlanShare/Program.cs @@ -44,8 +44,14 @@ created_at TEXT NOT NULL cmd.ExecuteNonQuery(); } +// --- Rate limiters (in-memory) --- +// Created before Build() so they can be DI-registered and swept by CleanupService. +var rateLimiter = new RateLimiter(maxRequests: 10, windowSeconds: 60); +var analyticsRateLimiter = new RateLimiter(maxRequests: 30, windowSeconds: 60); + // Register the cleanup background service builder.Services.AddSingleton(new PlanDbConfig(connectionString)); +builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter)); builder.Services.AddHostedService(); // Request size limit (10 MB) @@ -54,10 +60,6 @@ created_at TEXT NOT NULL var app = builder.Build(); app.UseCors(); -// --- Rate limiters (in-memory) --- -var rateLimiter = new RateLimiter(maxRequests: 10, windowSeconds: 60); -var analyticsRateLimiter = new RateLimiter(maxRequests: 30, windowSeconds: 60); - const int MaxTtlDays = 365; // --- Endpoints --- @@ -161,9 +163,15 @@ created_at TEXT NOT NULL return Results.BadRequest("Invalid JSON"); } - // Strip referrer to domain only (no full URLs with query params) - if (!string.IsNullOrEmpty(referrer) && Uri.TryCreate(referrer, UriKind.Absolute, out var refUri)) - referrer = refUri.Host; + // Strip referrer to domain only (no full URLs with query params). + // If it doesn't parse as an absolute URL, drop it — never persist raw + // client-supplied strings, since the dashboard renders referrers in HTML. + if (!string.IsNullOrEmpty(referrer)) + { + referrer = Uri.TryCreate(referrer, UriKind.Absolute, out var refUri) + ? refUri.Host + : null; + } // Visitor hash: SHA256(IP + User-Agent + date) — unique per day, no PII stored var ua = ctx.Request.Headers.UserAgent.FirstOrDefault() ?? ""; @@ -352,14 +360,18 @@ static string GenerateDeleteToken() record PlanDbConfig(string ConnectionString); +record RateLimiters(RateLimiter Share, RateLimiter Analytics); + sealed class CleanupService : BackgroundService { private readonly PlanDbConfig _config; + private readonly RateLimiters _rateLimiters; private readonly ILogger _logger; - public CleanupService(PlanDbConfig config, ILogger logger) + public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, ILogger logger) { _config = config; + _rateLimiters = rateLimiters; _logger = logger; } @@ -401,6 +413,14 @@ private void Cleanup() if (deleted > 0) _logger.LogInformation("Cleaned up {Count} old page views", deleted); } + + // Evict stale rate-limiter keys so the dictionary doesn't grow forever. + var shareEvicted = _rateLimiters.Share.Sweep(); + var analyticsEvicted = _rateLimiters.Analytics.Sweep(); + if (shareEvicted + analyticsEvicted > 0) + _logger.LogInformation( + "Evicted {Share} share + {Analytics} analytics rate-limit keys", + shareEvicted, analyticsEvicted); } catch (Exception ex) { @@ -441,4 +461,25 @@ public bool IsAllowed(string key) return true; } } + + /// + /// Evicts keys whose timestamp lists have gone empty. Call periodically + /// so the dictionary doesn't grow forever across unique IPs. + /// Returns the number of keys evicted. + /// + public int Sweep() + { + var cutoff = DateTime.UtcNow.AddSeconds(-_windowSeconds); + var evicted = 0; + foreach (var kvp in _requests) + { + lock (kvp.Value) + { + kvp.Value.RemoveAll(t => t < cutoff); + if (kvp.Value.Count == 0 && _requests.TryRemove(kvp)) + evicted++; + } + } + return evicted; + } } diff --git a/server/PlanShare/dashboard.html b/server/PlanShare/dashboard.html index b444890..f0a7fd5 100644 --- a/server/PlanShare/dashboard.html +++ b/server/PlanShare/dashboard.html @@ -275,13 +275,28 @@

Top Referrers (30 days)

} function updateReferrers() { var tbody = document.querySelector("#referrersTable tbody"); + tbody.replaceChildren(); if (!planStats || !planStats.traffic.top_referrers.length) { - tbody.innerHTML = "No referrer data yet"; + var tr = document.createElement("tr"); + var td = document.createElement("td"); + td.setAttribute("colspan", "2"); + td.style.color = "#484f58"; + td.textContent = "No referrer data yet"; + tr.appendChild(td); + tbody.appendChild(tr); return; } - tbody.innerHTML = planStats.traffic.top_referrers.map(function(r) { - return "" + r.referrer + "" + fmt(r.count) + ""; - }).join(""); + planStats.traffic.top_referrers.forEach(function(r) { + var tr = document.createElement("tr"); + var tdRef = document.createElement("td"); + tdRef.textContent = r.referrer; + var tdCount = document.createElement("td"); + tdCount.className = "num"; + tdCount.textContent = fmt(r.count); + tr.appendChild(tdRef); + tr.appendChild(tdCount); + tbody.appendChild(tr); + }); } function updateCharts() { updateChart("starsChart", "line", { diff --git a/src/PlanViewer.Core/Services/WindowsCredentialService.cs b/src/PlanViewer.Core/Services/WindowsCredentialService.cs index 2581f99..99c96aa 100644 --- a/src/PlanViewer.Core/Services/WindowsCredentialService.cs +++ b/src/PlanViewer.Core/Services/WindowsCredentialService.cs @@ -16,7 +16,7 @@ public bool SaveCredential(string serverId, string username, string password) userName: username, secret: password, comment: "planview credential", - persistence: CredentialPersistence.LocalMachine); + persistence: CredentialPersistence.Enterprise); return true; } catch { return false; }