fix(notify): neutralize markdown injection in slack/discord code blocks

slack and discord both wrap rendered findings in a triple-backtick
fence, but a finding's title is attacker-controlled (a scanned
target's page title, a crawled url, a cms name). a title embedding a
closing fence broke out of the block and injected live markdown
(mentions, masked links) into the channel. break up any backtick run
in the body before wrapping, and for slack additionally entity-escape
&, <, > per slack's api docs, since its parser resolves link/mention
syntax ahead of code-block boundaries.

Author: TBX3D

internal/notify/notify_test.go

@@ -230,7 +230,7 @@ func deadURL(t *testing.T) string {
 	t.Helper()
 	srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
 	u := srv.URL
-	srv.Close() // now nothing listens on that port
+	srv.Close()
 	return u
 }
 
@@ -275,3 +275,87 @@ func TestNotifyErrorRedactsTelegramToken(t *testing.T) {
 	}
 }
 
+// attacker-controlled finding content (a scanned target's page title, a
+// crawled url, a cms name) reaches the slack/discord code block verbatim. a
+// title that embeds a closing fence used to break out of our wrapping block
+// and inject live markdown (mentions, masked links) into the channel.
+func TestNotifyCodeBlockBreakoutNeutralized(t *testing.T) {
+	var c capture
+	srv := captureServer(t, &c)
+
+	evil := []finding.Finding{{
+		Target:   "https://evil.test",
+		Module:   "probe",
+		Severity: finding.SeverityHigh,
+		Key:      "probe:x",
+		Title:    "```\n@everyone pwned <https://evil.test|click>\n```",
+	}}
+	p := &discordProvider{webhook: srv.URL}
+	if err := p.send(context.Background(), srv.Client(), evil); err != nil {
+		t.Fatalf("send: %v", err)
+	}
+	var payload discordPayload
+	if err := json.Unmarshal(c.body, &payload); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	// a clean payload has exactly the 2 fences we added (open+close); any more
+	// means attacker content broke out.
+	if fences := strings.Count(payload.Content, "```"); fences > 2 {
+		t.Fatalf("INJECTION: attacker content added %d extra code fences, breaking out: %q", fences-2, payload.Content)
+	}
+}
+
+// slack resolves a bare "<...|...>" as a link/mention independent of code-block
+// boundaries, so the fence fix alone isn't enough for slack: the control
+// characters (&, <, >) must be entity-escaped too.
+func TestSlackEscapesControlChars(t *testing.T) {
+	var c capture
+	srv := captureServer(t, &c)
+
+	evil := []finding.Finding{{
+		Target:   "https://evil.test",
+		Module:   "probe",
+		Severity: finding.SeverityHigh,
+		Key:      "probe:x",
+		Title:    "<https://evil.test|click> & <!everyone>",
+	}}
+	p := &slackProvider{webhook: srv.URL}
+	if err := p.send(context.Background(), srv.Client(), evil); err != nil {
+		t.Fatalf("send: %v", err)
+	}
+	var payload slackPayload
+	if err := json.Unmarshal(c.body, &payload); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if strings.Contains(payload.Text, "<https://evil.test|click>") {
+		t.Fatalf("INJECTION: unescaped slack link syntax reached the payload: %q", payload.Text)
+	}
+	if !strings.Contains(payload.Text, "&lt;https://evil.test|click&gt;") || !strings.Contains(payload.Text, "&amp;") {
+		t.Fatalf("expected slack control chars entity-escaped, got: %q", payload.Text)
+	}
+}
+
+// robustness sanity: confirm a zero http.Client.Timeout would mean an
+// unbounded client (not a bug in notify per se, but documents that ctx - not
+// Timeout - is what bounds a hung endpoint here).
+func TestNotifyZeroTimeoutIsUnbounded(t *testing.T) {
+	blocked := make(chan struct{})
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		<-blocked
+	}))
+	t.Cleanup(func() { close(blocked); srv.Close() })
+
+	ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
+	defer cancel()
+	p := &slackProvider{webhook: srv.URL}
+	done := make(chan error, 1)
+	go func() { done <- p.send(ctx, srv.Client(), sampleFindings()) }()
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("expected ctx-cancel error from hung endpoint")
+		}
+	case <-time.After(3 * time.Second):
+		t.Fatal("send did not honor ctx cancellation on hung endpoint")
+	}
+}

internal/notify/slack.go

@@ -15,6 +15,7 @@ package notify
 import (
 	"context"
 	"net/http"
+	"strings"
 
 	"github.com/vmfunc/sif/internal/finding"
 )
@@ -34,12 +35,35 @@ type slackPayload struct {
 }
 
 func (s *slackProvider) send(ctx context.Context, client *http.Client, findings []finding.Finding) error {
-	payload := slackPayload{Text: codeBlock(renderFindings(findings))}
+	payload := slackPayload{Text: codeBlock(escapeSlackText(renderFindings(findings)))}
 	return postJSON(ctx, client, s.webhook, payload)
 }
 
+// escapeSlackText entity-escapes slack's three control characters ahead of any
+// other formatting, per slack's api docs: & first (so the escapes below don't
+// double-escape), then < and >. slack's parser resolves a bare "<...|...>" as
+// a link/mention regardless of surrounding code-fence text, so a scanned
+// target's title (e.g. "<https://evil.test|click>") would otherwise render as
+// a live masked link.
+func escapeSlackText(body string) string {
+	r := strings.NewReplacer("&", "&amp;", "<", "&lt;", ">", "&gt;")
+	return r.Replace(body)
+}
+
 // codeBlock wraps body in a triple-backtick fence; both slack and discord render
-// it fixed-width, which preserves the column-aligned finding lines.
+// it fixed-width, which preserves the column-aligned finding lines. body runs
+// through sanitizeFence first so attacker-controlled finding content (a title
+// pulled from the scanned target) can't close the fence early and inject
+// markdown/mentions outside it.
 func codeBlock(body string) string {
-	return "```\n" + body + "```"
+	return "```\n" + sanitizeFence(body) + "```"
+}
+
+// sanitizeFence breaks up any triple-backtick run inside body by interleaving
+// zero-width spaces between the backticks. the text still reads as backticks
+// to a human but neither slack nor discord treats it as a fence boundary, so
+// it can't prematurely close the code block we wrap it in.
+func sanitizeFence(body string) string {
+	const zwsp = "\u200b" // zero-width space
+	return strings.ReplaceAll(body, "```", "`"+zwsp+"`"+zwsp+"`")
 }