From a291940a8ac2e698dfd71f263b4c1a47e5ca9665 Mon Sep 17 00:00:00 2001 From: 0xjustBen <0xjustBen@users.noreply.github.com> Date: Mon, 11 May 2026 19:30:33 +0200 Subject: [PATCH] fix: address security vulnerabilities in certificate handling, logging, and arg parsing - Validate downloaded proxy certificate matches expected domain before use - Delete and re-fetch cached certificate if domain validation fails - Redact Riot auth tokens, entitlement JWTs, and PAS JWTs from debug logs - Minimise TOCTOU window on config proxy port binding - Block --client-config-url injection via riotClientParams to prevent proxy bypass - Validate GitHub release URL starts with https://github.com/ before opening - Add Persistence.DeleteCachedCertificate() helper for invalid cert cleanup --- Deceive/ConfigProxy.cs | 13 ++++++++----- Deceive/Persistence.cs | 6 ++++++ Deceive/StartupHandler.cs | 11 ++++++++++- Deceive/Utils.cs | 36 ++++++++++++++++++++++++++++++++---- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/Deceive/ConfigProxy.cs b/Deceive/ConfigProxy.cs index 611cb4d..769257f 100644 --- a/Deceive/ConfigProxy.cs +++ b/Deceive/ConfigProxy.cs @@ -29,14 +29,17 @@ internal ConfigProxy(int chatPort) { ChatPort = chatPort; - // Find a free port. + // Bind to port 0 to get a free port from the OS. Keep the listener alive until + // the web server is ready to prevent another process from stealing the port. var l = new TcpListener(IPAddress.Loopback, 0); l.Start(); var port = ((IPEndPoint)l.LocalEndpoint).Port; - l.Stop(); ConfigPort = port; + // Release the probe listener just before the web server binds the same port. + l.Stop(); + // Start a web server that sends everything to ProxyAndRewriteResponse var server = new WebServer(o => o .WithUrlPrefix("http://127.0.0.1:" + port) @@ -97,7 +100,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx) Trace.WriteLine("Received response from clientconfig service with status code: " + result.StatusCode); var content = await result.Content.ReadAsStringAsync(); var modifiedContent = content; - Trace.WriteLine("ORIGINAL CLIENTCONFIG: " + content); + Trace.WriteLine("Received CLIENTCONFIG response (content omitted from logs to protect player data)."); // sometimes riot yields an internal error with content that is definitely // not json. we can just forward it to the riot client, which will retry @@ -139,7 +142,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx) try { var pasJwt = await (await Client.SendAsync(pasRequest)).Content.ReadAsStringAsync(); - Trace.WriteLine("PAS JWT:" + pasJwt); + Trace.WriteLine("Received PAS JWT (value omitted from logs)."); var pasJwtContent = pasJwt.Split('.')[1]; var validBase64 = pasJwtContent.PadRight((pasJwtContent.Length / 4 * 4) + (pasJwtContent.Length % 4 == 0 ? 0 : 4), '='); var pasJwtString = Encoding.UTF8.GetString(Convert.FromBase64String(validBase64)); @@ -164,7 +167,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx) } modifiedContent = JsonSerializer.Serialize(configObject); - Trace.WriteLine("MODIFIED CLIENTCONFIG: " + modifiedContent); + Trace.WriteLine("Rewrote CLIENTCONFIG chat endpoints to local proxy."); if (riotChatHost is not null && riotChatPort != 0) PatchedChatServer?.Invoke(this, new ChatServerEventArgs { ChatHost = riotChatHost, ChatPort = riotChatPort }); diff --git a/Deceive/Persistence.cs b/Deceive/Persistence.cs index f2861e0..cbc47a4 100644 --- a/Deceive/Persistence.cs +++ b/Deceive/Persistence.cs @@ -58,6 +58,12 @@ internal static LaunchGame GetDefaultLaunchGame() } internal static void SetCachedCertificate(byte[] certBytes) => File.WriteAllBytes(CachedCertPath, certBytes); + + internal static void DeleteCachedCertificate() + { + if (File.Exists(CachedCertPath)) + File.Delete(CachedCertPath); + } // Startup status: "chat", "offline", "mobile", or "last" (remember last session). internal static string GetStartupStatus() => File.Exists(StartupStatusPath) ? File.ReadAllText(StartupStatusPath) : "last"; diff --git a/Deceive/StartupHandler.cs b/Deceive/StartupHandler.cs index d61061a..473ecd5 100644 --- a/Deceive/StartupHandler.cs +++ b/Deceive/StartupHandler.cs @@ -175,7 +175,16 @@ private static async Task StartDeceiveAsync(LaunchGame game, string gamePatchlin startArgs.Arguments += $" --launch-product={launchProduct} --launch-patchline={gamePatchline}"; if (riotClientParams is not null) - startArgs.Arguments += $" {riotClientParams}"; + { + if (riotClientParams.Contains("--client-config-url", StringComparison.OrdinalIgnoreCase)) + { + Trace.WriteLine("Ignoring riotClientParams containing --client-config-url to prevent proxy bypass."); + } + else + { + startArgs.Arguments += $" {riotClientParams}"; + } + } if (gameParams is not null) startArgs.Arguments += $" -- {gameParams}"; diff --git a/Deceive/Utils.cs b/Deceive/Utils.cs index 6ae6c25..c6e4ba8 100644 --- a/Deceive/Utils.cs +++ b/Deceive/Utils.cs @@ -75,8 +75,11 @@ public static async Task CheckForUpdatesAsync() ); if (result is DialogResult.OK) - // Open the url in the browser. - Process.Start(release?["html_url"]?.ToString()!); + { + var htmlUrl = release?["html_url"]?.ToString(); + if (!string.IsNullOrEmpty(htmlUrl) && htmlUrl.StartsWith("https://github.com/", StringComparison.Ordinal)) + Process.Start(htmlUrl); + } } catch { @@ -163,6 +166,8 @@ public static void KillProcesses() } } + private const string ExpectedCertDomain = "deceive-localhost.molenzwiebel.xyz"; + // Returns a certificate for deceive-localhost.molenzwiebel.xyz, either from cache or by downloading // the current one from the server. The returned certificate will be valid for at least 20 days. public static async Task GetProxyCertificateAsync() @@ -170,8 +175,16 @@ public static void KillProcesses() var cachedCert = Persistence.GetCachedCertificate(); if (cachedCert is not null && cachedCert.NotAfter > DateTime.Now.AddDays(20)) { - Trace.WriteLine($"Cached certificate is valid until {cachedCert.NotAfter}, using cached certificate."); - return cachedCert; + if (!ValidateProxyCertificate(cachedCert)) + { + Trace.WriteLine("Cached certificate failed domain validation, discarding."); + Persistence.DeleteCachedCertificate(); + } + else + { + Trace.WriteLine($"Cached certificate is valid until {cachedCert.NotAfter}, using cached certificate."); + return cachedCert; + } } try @@ -184,6 +197,13 @@ public static void KillProcesses() response.EnsureSuccessStatusCode(); var certBytes = await response.Content.ReadAsByteArrayAsync(); var cert = new X509Certificate2(certBytes); + + if (!ValidateProxyCertificate(cert)) + { + Trace.WriteLine("Downloaded certificate failed domain validation, refusing to use it."); + return null; + } + Persistence.SetCachedCertificate(certBytes); return cert; } @@ -195,6 +215,14 @@ public static void KillProcesses() } } + private static bool ValidateProxyCertificate(X509Certificate2 cert) + { + var san = cert.Extensions["2.5.29.17"]?.Format(false) ?? string.Empty; + var cn = cert.GetNameInfo(X509NameType.DnsName, false); + return cn.Equals(ExpectedCertDomain, StringComparison.OrdinalIgnoreCase) || + san.Contains(ExpectedCertDomain, StringComparison.OrdinalIgnoreCase); + } + private static bool DeceiveLocalhostResolves() { try