diff --git a/internal/report/markdown.go b/internal/report/markdown.go index 44b1bb1..6dab2c2 100644 --- a/internal/report/markdown.go +++ b/internal/report/markdown.go @@ -40,7 +40,7 @@ func Markdown(results []Result) []byte { for i := 0; i < len(order); i++ { target := order[i] b.WriteString("## ") - b.WriteString(target) + b.WriteString(sanitizeHeading(target)) b.WriteString("\n\n") mods := byTarget[target] @@ -49,7 +49,7 @@ func Markdown(results []Result) []byte { for j := 0; j < len(mods); j++ { b.WriteString("### ") - b.WriteString(mods[j].Module) + b.WriteString(sanitizeHeading(mods[j].Module)) b.WriteString("\n\n") b.WriteString("```json\n") b.WriteString(prettyJSON(mods[j].Data)) @@ -60,6 +60,17 @@ func Markdown(results []Result) []byte { return []byte(b.String()) } +// sanitizeHeading strips CR/LF from operator-supplied text (targets, module +// ids) before it's written into a markdown heading line. both target and +// module id can come from scan input, so an embedded newline followed by +// "## ..." would otherwise render as a standalone, injected heading. +func sanitizeHeading(s string) string { + s = strings.ReplaceAll(s, "\r\n", " ") + s = strings.ReplaceAll(s, "\r", " ") + s = strings.ReplaceAll(s, "\n", " ") + return s +} + // prettyJSON re-indents the raw finding for readability; if it doesn't parse as // json (shouldn't happen, but never trust it) the raw bytes are returned as-is. func prettyJSON(raw json.RawMessage) string { diff --git a/internal/report/report_test.go b/internal/report/report_test.go index c0a636f..4406a7a 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -14,6 +14,8 @@ package report import ( "encoding/json" + "reflect" + "sort" "strings" "testing" ) @@ -75,6 +77,46 @@ func TestSARIF_ValidAndContainsFindings(t *testing.T) { } } +func TestSARIF_RulesAreSorted(t *testing.T) { + // rules are collected from a map internally, so without an explicit sort + // the emitted order is whatever the map iteration happened to produce. + // several distinct module ids make an accidentally-sorted iteration + // vanishingly unlikely, so this pins the fix rather than relying on luck. + results := []Result{ + {Target: "https://t", Module: "zeta", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "mike", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "alpha", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "yankee", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "bravo", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "delta", Data: json.RawMessage(`{}`)}, + {Target: "https://t", Module: "kilo", Data: json.RawMessage(`{}`)}, + } + + var firstIDs []string + for run := 0; run < 5; run++ { + out, err := SARIF(results) + if err != nil { + t.Fatalf("SARIF: %v", err) + } + var doc sarifLog + if err := json.Unmarshal(out, &doc); err != nil { + t.Fatalf("invalid json: %v", err) + } + ids := make([]string, len(doc.Runs[0].Tool.Driver.Rules)) + for i, r := range doc.Runs[0].Tool.Driver.Rules { + ids[i] = r.ID + } + if !sort.StringsAreSorted(ids) { + t.Fatalf("run %d: driver.rules not sorted: %v", run, ids) + } + if run == 0 { + firstIDs = ids + } else if !reflect.DeepEqual(firstIDs, ids) { + t.Fatalf("driver.rules order changed across runs: %v vs %v", firstIDs, ids) + } + } +} + func TestSARIF_DedupesRulesAcrossTargets(t *testing.T) { // the same module on two targets must yield one rule but two results. results := []Result{ @@ -193,6 +235,24 @@ func TestMarkdown_GroupsByTarget(t *testing.T) { } } +func TestMarkdown_StripsNewlinesFromTargetHeader(t *testing.T) { + // same newline-injection guard as sanitizeHeading (markdown.go); this exercises the malicious-input case. + results := []Result{ + {Target: "https://evil.example.com\n## injected", Module: "probe", Data: json.RawMessage(`{"status_code":200}`)}, + } + out := string(Markdown(results)) + + lines := strings.Split(out, "\n") + for _, line := range lines { + if line == "## injected" { + t.Errorf("target newline produced a standalone injected heading:\n%s", out) + } + } + if strings.Contains(out, "\r") { + t.Errorf("markdown output must not contain carriage returns:\n%q", out) + } +} + // sarifHasResult reports whether any result carries the given rule id and target // uri, the pairing that proves a finding survived serialization. func sarifHasResult(results []sarifResult, ruleID, target string) bool { diff --git a/internal/report/sarif.go b/internal/report/sarif.go index 2452e1a..76f657c 100644 --- a/internal/report/sarif.go +++ b/internal/report/sarif.go @@ -15,6 +15,7 @@ package report import ( "encoding/json" "fmt" + "sort" ) // sarif format/version constants pinned to the 2.1.0 schema so the output is @@ -121,9 +122,18 @@ func SARIF(results []Result) ([]byte, error) { } // rules must list each id exactly once; build it from the set so duplicate - // modules across targets don't duplicate the rule. - rules := make([]sarifRule, 0, len(ruleSet)) + // modules across targets don't duplicate the rule. the set itself ranges + // in random map order, so sort the ids first: otherwise driver.rules + // would come out in a different order on every run, making the sarif + // output byte-unstable across identical scans. + ruleIDs := make([]string, 0, len(ruleSet)) for id := range ruleSet { + ruleIDs = append(ruleIDs, id) + } + sort.Strings(ruleIDs) + + rules := make([]sarifRule, 0, len(ruleIDs)) + for _, id := range ruleIDs { rules = append(rules, sarifRule{ID: id}) }