Skip to content

Commit 4de8fae

Browse files
committed
fix(notify): break every backtick so long runs cannot reform a fence
replacing exact triples left a trailing bare backtick, so a run whose length is 2 mod 3 (5, 8, 11...) came back out with three adjacent backticks and reopened the block. discord takes the sanitized body with no entity escaping, so that was a live breakout for mentions and masked links. the breakout test only covered n=3; it now walks 3,4,5,6,8,11 and fails on the old behaviour at n=5.
1 parent 7d938ac commit 4de8fae

2 files changed

Lines changed: 32 additions & 24 deletions

File tree

internal/notify/notify_test.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -278,25 +278,30 @@ func TestNotifyCodeBlockBreakoutNeutralized(t *testing.T) {
278278
var c capture
279279
srv := captureServer(t, &c)
280280

281-
evil := []finding.Finding{{
282-
Target: "https://evil.test",
283-
Module: "probe",
284-
Severity: finding.SeverityHigh,
285-
Key: "probe:x",
286-
Title: "```\n@everyone pwned <https://evil.test|click>\n```",
287-
}}
288-
p := &discordProvider{webhook: srv.URL}
289-
if err := p.send(context.Background(), srv.Client(), evil); err != nil {
290-
t.Fatalf("send: %v", err)
291-
}
292-
var payload discordPayload
293-
if err := json.Unmarshal(c.body, &payload); err != nil {
294-
t.Fatalf("unmarshal: %v", err)
295-
}
296-
// a clean payload has exactly the 2 fences we added (open+close); any more
297-
// means attacker content broke out.
298-
if fences := strings.Count(payload.Content, "```"); fences > 2 {
299-
t.Fatalf("INJECTION: attacker content added %d extra code fences, breaking out: %q", fences-2, payload.Content)
281+
// a backtick run of any length has to come out broken; 5, 8 and 11 are the
282+
// lengths that reformed a fence when only exact triples were replaced.
283+
for _, n := range []int{3, 4, 5, 6, 8, 11} {
284+
run := strings.Repeat("`", n)
285+
evil := []finding.Finding{{
286+
Target: "https://evil.test",
287+
Module: "probe",
288+
Severity: finding.SeverityHigh,
289+
Key: "probe:x",
290+
Title: run + "\n@everyone pwned <https://evil.test|click>\n" + run,
291+
}}
292+
p := &discordProvider{webhook: srv.URL}
293+
if err := p.send(context.Background(), srv.Client(), evil); err != nil {
294+
t.Fatalf("run of %d: send: %v", n, err)
295+
}
296+
var payload discordPayload
297+
if err := json.Unmarshal(c.body, &payload); err != nil {
298+
t.Fatalf("run of %d: unmarshal: %v", n, err)
299+
}
300+
// a clean payload has exactly the 2 fences we added (open+close); any more
301+
// means attacker content broke out.
302+
if fences := strings.Count(payload.Content, "```"); fences > 2 {
303+
t.Fatalf("INJECTION: run of %d backticks added %d extra code fences, breaking out: %q", n, fences-2, payload.Content)
304+
}
300305
}
301306
}
302307

internal/notify/slack.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,14 @@ func codeBlock(body string) string {
5757
return "```\n" + sanitizeFence(body) + "```"
5858
}
5959

60-
// sanitizeFence breaks up any triple-backtick run inside body by interleaving
61-
// zero-width spaces between the backticks. the text still reads as backticks
62-
// to a human but neither slack nor discord treats it as a fence boundary, so
63-
// it can't prematurely close the code block we wrap it in.
60+
// sanitizeFence separates every backtick in body from the next with a
61+
// zero-width space. the text still reads as backticks to a human but no two
62+
// are ever adjacent, so neither slack nor discord sees a fence boundary and
63+
// attacker content can't close the code block we wrap it in.
64+
//
65+
// breaking exact triples instead would leave a trailing bare backtick, and any
66+
// run of length \u2261 2 mod 3 (5, 8, 11...) would reform a contiguous triple.
6467
func sanitizeFence(body string) string {
6568
const zwsp = "\u200b" // zero-width space
66-
return strings.ReplaceAll(body, "```", "`"+zwsp+"`"+zwsp+"`")
69+
return strings.ReplaceAll(body, "`", "`"+zwsp)
6770
}

0 commit comments

Comments
 (0)