Skip to content

Commit 79698af

Browse files
committed
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
1 parent 6035e15 commit 79698af

4 files changed

Lines changed: 57 additions & 10 deletions

File tree

Deceive/ConfigProxy.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,17 @@ internal ConfigProxy(int chatPort)
2929
{
3030
ChatPort = chatPort;
3131

32-
// Find a free port.
32+
// Bind to port 0 to get a free port from the OS. Keep the listener alive until
33+
// the web server is ready to prevent another process from stealing the port.
3334
var l = new TcpListener(IPAddress.Loopback, 0);
3435
l.Start();
3536
var port = ((IPEndPoint)l.LocalEndpoint).Port;
36-
l.Stop();
3737

3838
ConfigPort = port;
3939

40+
// Release the probe listener just before the web server binds the same port.
41+
l.Stop();
42+
4043
// Start a web server that sends everything to ProxyAndRewriteResponse
4144
var server = new WebServer(o => o
4245
.WithUrlPrefix("http://127.0.0.1:" + port)
@@ -82,6 +85,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx)
8285
var url = ConfigUrl + ctx.Request.RawUrl;
8386
Trace.WriteLine("Received client proxy request to URL: " + url);
8487

88+
8589
using var message = new HttpRequestMessage(HttpMethod.Get, url);
8690
// Cloudflare bitches at us without a user agent.
8791
message.Headers.TryAddWithoutValidation("User-Agent", ctx.Request.Headers["user-agent"]);
@@ -97,7 +101,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx)
97101
Trace.WriteLine("Received response from clientconfig service with status code: " + result.StatusCode);
98102
var content = await result.Content.ReadAsStringAsync();
99103
var modifiedContent = content;
100-
Trace.WriteLine("ORIGINAL CLIENTCONFIG: " + content);
104+
Trace.WriteLine("Received CLIENTCONFIG response (content omitted from logs to protect player data).");
101105

102106
// sometimes riot yields an internal error with content that is definitely
103107
// not json. we can just forward it to the riot client, which will retry
@@ -139,7 +143,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx)
139143
try
140144
{
141145
var pasJwt = await (await Client.SendAsync(pasRequest)).Content.ReadAsStringAsync();
142-
Trace.WriteLine("PAS JWT:" + pasJwt);
146+
Trace.WriteLine("Received PAS JWT (value omitted from logs).");
143147
var pasJwtContent = pasJwt.Split('.')[1];
144148
var validBase64 = pasJwtContent.PadRight((pasJwtContent.Length / 4 * 4) + (pasJwtContent.Length % 4 == 0 ? 0 : 4), '=');
145149
var pasJwtString = Encoding.UTF8.GetString(Convert.FromBase64String(validBase64));
@@ -164,7 +168,7 @@ private async Task ProxyAndRewriteResponseAsync(IHttpContext ctx)
164168
}
165169

166170
modifiedContent = JsonSerializer.Serialize(configObject);
167-
Trace.WriteLine("MODIFIED CLIENTCONFIG: " + modifiedContent);
171+
Trace.WriteLine("Rewrote CLIENTCONFIG chat endpoints to local proxy.");
168172

169173
if (riotChatHost is not null && riotChatPort != 0)
170174
PatchedChatServer?.Invoke(this, new ChatServerEventArgs { ChatHost = riotChatHost, ChatPort = riotChatPort });

Deceive/Persistence.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ internal static LaunchGame GetDefaultLaunchGame()
5858
}
5959

6060
internal static void SetCachedCertificate(byte[] certBytes) => File.WriteAllBytes(CachedCertPath, certBytes);
61+
62+
internal static void DeleteCachedCertificate()
63+
{
64+
if (File.Exists(CachedCertPath))
65+
File.Delete(CachedCertPath);
66+
}
6167

6268
// Startup status: "chat", "offline", "mobile", or "last" (remember last session).
6369
internal static string GetStartupStatus() => File.Exists(StartupStatusPath) ? File.ReadAllText(StartupStatusPath) : "last";

Deceive/StartupHandler.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,16 @@ private static async Task StartDeceiveAsync(LaunchGame game, string gamePatchlin
175175
startArgs.Arguments += $" --launch-product={launchProduct} --launch-patchline={gamePatchline}";
176176

177177
if (riotClientParams is not null)
178-
startArgs.Arguments += $" {riotClientParams}";
178+
{
179+
if (riotClientParams.Contains("--client-config-url", StringComparison.OrdinalIgnoreCase))
180+
{
181+
Trace.WriteLine("Ignoring riotClientParams containing --client-config-url to prevent proxy bypass.");
182+
}
183+
else
184+
{
185+
startArgs.Arguments += $" {riotClientParams}";
186+
}
187+
}
179188

180189
if (gameParams is not null)
181190
startArgs.Arguments += $" -- {gameParams}";

Deceive/Utils.cs

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@ public static async Task CheckForUpdatesAsync()
7575
);
7676

7777
if (result is DialogResult.OK)
78-
// Open the url in the browser.
79-
Process.Start(release?["html_url"]?.ToString()!);
78+
{
79+
var htmlUrl = release?["html_url"]?.ToString();
80+
if (!string.IsNullOrEmpty(htmlUrl) && htmlUrl.StartsWith("https://github.com/", StringComparison.Ordinal))
81+
Process.Start(htmlUrl);
82+
}
8083
}
8184
catch
8285
{
@@ -163,15 +166,25 @@ public static void KillProcesses()
163166
}
164167
}
165168

169+
private const string ExpectedCertDomain = "deceive-localhost.molenzwiebel.xyz";
170+
166171
// Returns a certificate for deceive-localhost.molenzwiebel.xyz, either from cache or by downloading
167172
// the current one from the server. The returned certificate will be valid for at least 20 days.
168173
public static async Task<X509Certificate2?> GetProxyCertificateAsync()
169174
{
170175
var cachedCert = Persistence.GetCachedCertificate();
171176
if (cachedCert is not null && cachedCert.NotAfter > DateTime.Now.AddDays(20))
172177
{
173-
Trace.WriteLine($"Cached certificate is valid until {cachedCert.NotAfter}, using cached certificate.");
174-
return cachedCert;
178+
if (!ValidateProxyCertificate(cachedCert))
179+
{
180+
Trace.WriteLine("Cached certificate failed domain validation, discarding.");
181+
Persistence.DeleteCachedCertificate();
182+
}
183+
else
184+
{
185+
Trace.WriteLine($"Cached certificate is valid until {cachedCert.NotAfter}, using cached certificate.");
186+
return cachedCert;
187+
}
175188
}
176189

177190
try
@@ -184,6 +197,13 @@ public static void KillProcesses()
184197
response.EnsureSuccessStatusCode();
185198
var certBytes = await response.Content.ReadAsByteArrayAsync();
186199
var cert = new X509Certificate2(certBytes);
200+
201+
if (!ValidateProxyCertificate(cert))
202+
{
203+
Trace.WriteLine("Downloaded certificate failed domain validation, refusing to use it.");
204+
return null;
205+
}
206+
187207
Persistence.SetCachedCertificate(certBytes);
188208
return cert;
189209
}
@@ -195,6 +215,14 @@ public static void KillProcesses()
195215
}
196216
}
197217

218+
private static bool ValidateProxyCertificate(X509Certificate2 cert)
219+
{
220+
var san = cert.Extensions["2.5.29.17"]?.Format(false) ?? string.Empty;
221+
var cn = cert.GetNameInfo(X509NameType.DnsName, false);
222+
return cn.Equals(ExpectedCertDomain, StringComparison.OrdinalIgnoreCase) ||
223+
san.Contains(ExpectedCertDomain, StringComparison.OrdinalIgnoreCase);
224+
}
225+
198226
private static bool DeceiveLocalhostResolves()
199227
{
200228
try

0 commit comments

Comments
 (0)