Sanitize every field the channel summary quotes, not just the handle

The endpoint is public and unauthenticated, so the whole uploaded file is
attacker-chosen, not only the part the client copied from Ja2.ini. The build
field sat inside backticks a backtick closes, and the access-violation text
went in raw, so either could carry markdown or a link into the channel.

One clean(): printable ASCII minus what Discord reads as markup or a URL,
length-capped. It replaces the handle's own stripping and absorbs the .trim()
the field getters did, which also drops the CR that "(.+)$" captures off a
CRLF report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
majcosta
2026-07-28 10:25:47 -03:00
committed by majcosta
co-authored by Claude Opus 5
parent 90f5bff69b
commit 1d0b4beb4c
3 changed files with 24 additions and 9 deletions
+6 -2
View File
@@ -96,6 +96,10 @@ a millisecond of CPU — the time spent waiting on Discord is not metered.
No authentication. The endpoint is public and its URL ships in every player's No authentication. The endpoint is public and its URL ships in every player's
`Ja2.ini`, so assume it will eventually be found; the size and `*** CRASH` checks `Ja2.ini`, so assume it will eventually be found; the size and `*** CRASH` checks
only keep out drive-by scanners. Report contents are attacker-controlled text, which only keep out drive-by scanners. Every field the summary line quotes is therefore
is why the summary line strips markdown from the handle and sends attacker-chosen, not just the handle: all of them go through `clean()`, which keeps
printable ASCII minus Discord's markup and link characters, and the post sends
`allowed_mentions: {parse: []}`. Blast radius of abuse is a message we delete. `allowed_mentions: {parse: []}`. Blast radius of abuse is a message we delete.
The per-IP limiter does nothing against a distributed flood — that would cost
channel noise and the 100k/day request budget, not money.
+6 -1
View File
@@ -7,7 +7,7 @@ const REPORT = `
time 2026-07-26 10:41:02 UTC time 2026-07-26 10:41:02 UTC
build 6a941c06 build 6a941c06
handle @marco*evil handle @marco*evil
access violation: read from 00000002 access violation: read from 00000002 [x](https://evil.test) \`
[0] 0071D5A0 [0] 0071D5A0
[1] 006BE7DE [1] 006BE7DE
`; `;
@@ -33,6 +33,11 @@ assert.match(content, /C0000005/);
assert.match(content, /read from 00000002/); assert.match(content, /read from 00000002/);
assert.match(content, /build `6a941c06`/); assert.match(content, /build `6a941c06`/);
assert.match(content, /marcoevil/); // markdown and @ stripped from the handle assert.match(content, /marcoevil/); // markdown and @ stripped from the handle
// every field is attacker-chosen: no field may carry markup or a link into the
// channel, and none may close the backticks or bold the summary wraps it in.
assert.ok(!content.includes("]("), content); // no link syntax out of the report
assert.ok(!content.includes("://"), content); // and no bare URL either
assert.equal(content.match(/`/g).length, 4); // only the two pairs summarize() opens
assert.deepEqual(JSON.parse(sent.body.get("payload_json")).allowed_mentions, { parse: [] }); assert.deepEqual(JSON.parse(sent.body.get("payload_json")).allowed_mentions, { parse: [] });
assert.equal(await sent.body.get("files[0]").text(), REPORT); assert.equal(await sent.body.get("files[0]").text(), REPORT);
+12 -6
View File
@@ -15,17 +15,23 @@
const MAX_BYTES = 64 * 1024; // reports run 2-8 KB; anything near this is not ours const MAX_BYTES = 64 * 1024; // reports run 2-8 KB; anything near this is not ours
// Every field below is lifted out of the uploaded file, and the endpoint is public
// and unauthenticated: treat all of it as attacker-chosen, not just the handle.
// Keep printable ASCII minus everything Discord reads as markup or a link, and cap
// the length, so nothing in a report can shape the message around it.
const clean = (s) => (s || "").replace(/[^A-Za-z0-9 .,+-]/g, "").slice(0, 64).trim();
// Pull the header fields the client writes, for a one-line channel summary. // Pull the header fields the client writes, for a one-line channel summary.
// See writeExceptionBacktrace(): "*** CRASH code=... ***", then " <key> <value>". // See writeExceptionBacktrace(): "*** CRASH code=... ***", then " <key> <value>".
function summarize(text) { function summarize(text) {
const field = (name) => (text.match(new RegExp(`^\\s+${name} (.+)$`, "m")) || [])[1]?.trim(); const field = (name) => (text.match(new RegExp(`^\\s+${name} (.+)$`, "m")) || [])[1];
const code = (text.match(/code=(\w+)/) || [])[1] || "????????"; const code = clean((text.match(/code=(\w+)/) || [])[1]) || "????????";
const av = (text.match(/access violation: (.+)/) || [])[1]?.trim(); const av = clean((text.match(/access violation: (.+)/) || [])[1]);
const parts = [`\`${code}\`${av ? ` (${av})` : ""}`]; const parts = [`\`${code}\`${av ? ` (${av})` : ""}`];
const build = field("build"); const build = clean(field("build"));
const handle = field("handle"); const handle = clean(field("handle"));
if (build) parts.push(`build \`${build}\``); if (build) parts.push(`build \`${build}\``);
if (handle) parts.push(`from **${handle.replace(/[`*_~|@]/g, "")}**`); // no pings, no markdown if (handle) parts.push(`from **${handle}**`);
return parts.join(" · "); return parts.join(" · ");
} }