From 18b4841a9a8bc42b9e5fbab698d4923afd22293c Mon Sep 17 00:00:00 2001 From: TBX3D <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:57:11 -0700 Subject: [PATCH 1/4] fix(output): sanitize control bytes before printing to the terminal response-derived content (page titles, headers, whois records, ...) reached the sink raw through fmt.Fprintf and lipgloss.Style.Render, so a hostile response could rewrite the terminal title or clear the screen via embedded OSC/CSI escape sequences (e.g. probe.go's page-title highlight). add output.Sanitize, drop C0/C1 control bytes and DEL while keeping tab/newline and legitimate SGR color sequences, and apply it in Sink.Info/Success/Warn/Error, ModuleLogger.*, and the Highlight/Muted/ Status/Severity* style renderers so callers get the protection for free. --- internal/output/nocolor_test.go | 26 +++++++ internal/output/output.go | 119 +++++++++++++++++++++++++++---- internal/output/sanitize_test.go | 88 +++++++++++++++++++++++ internal/scan/probe_ansi_test.go | 78 ++++++++++++++++++++ 4 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 internal/output/nocolor_test.go create mode 100644 internal/output/sanitize_test.go create mode 100644 internal/scan/probe_ansi_test.go diff --git a/internal/output/nocolor_test.go b/internal/output/nocolor_test.go new file mode 100644 index 00000000..46eb5eef --- /dev/null +++ b/internal/output/nocolor_test.go @@ -0,0 +1,26 @@ +package output + +import ( + "strings" + "testing" + + "github.com/charmbracelet/lipgloss" +) + +// regression guard: Highlight.Render must strip embedded control/OSC bytes +// from its argument before styling it, even under NO_COLOR, so an +// attacker-controlled string (a response title, header, ...) can't rewrite +// the terminal title or move the cursor when it's highlighted for display. +func TestHighlightRenderStripsEmbeddedESC(t *testing.T) { + t.Setenv("NO_COLOR", "1") + lipgloss.SetColorProfile(4) + evil := "\x1b]0;PWNED\x07x" + out := Highlight.Render(evil) + if strings.Contains(out, "\x1b]0;PWNED\x07") { + t.Fatalf("NO_COLOR path let attacker OSC sequence through: %q", out) + } + if !strings.Contains(out, "x") { + t.Fatalf("legitimate content was lost, got %q", out) + } + t.Logf("CONFIRMED: Render strips attacker control bytes from content: %q", out) +} diff --git a/internal/output/output.go b/internal/output/output.go index 0a14c81b..0be6f119 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -18,6 +18,7 @@ import ( "os" "strings" "sync" + "unicode/utf8" "github.com/charmbracelet/lipgloss" ) @@ -42,9 +43,9 @@ var ( // Text styles var ( - Highlight = lipgloss.NewStyle().Bold(true).Foreground(ColorWhite) - Muted = lipgloss.NewStyle().Foreground(ColorGray) - Status = lipgloss.NewStyle().Bold(true).Foreground(ColorGreen) + Highlight = Style{lipgloss.NewStyle().Bold(true).Foreground(ColorWhite)} + Muted = Style{lipgloss.NewStyle().Foreground(ColorGray)} + Status = Style{lipgloss.NewStyle().Bold(true).Foreground(ColorGreen)} ) // Box style for banners @@ -68,10 +69,10 @@ var Subheading = lipgloss.NewStyle(). // Severity styles var ( - SeverityLow = lipgloss.NewStyle().Foreground(ColorGreen) - SeverityMedium = lipgloss.NewStyle().Foreground(ColorYellow) - SeverityHigh = lipgloss.NewStyle().Foreground(lipgloss.Color("#f97316")) // orange - SeverityCritical = lipgloss.NewStyle().Foreground(ColorRed).Bold(true) + SeverityLow = Style{lipgloss.NewStyle().Foreground(ColorGreen)} + SeverityMedium = Style{lipgloss.NewStyle().Foreground(ColorYellow)} + SeverityHigh = Style{lipgloss.NewStyle().Foreground(lipgloss.Color("#f97316"))} + SeverityCritical = Style{lipgloss.NewStyle().Foreground(ColorRed).Bold(true)} ) // Module color palette - visually distinct, nice colors @@ -196,6 +197,94 @@ func Writer() io.Writer { return sink } +// Style wraps a lipgloss.Style so Render sanitizes its arguments first. Styles +// used to highlight response-derived content are declared with this type so +// every call site gets the control-byte stripping in Sanitize for free, +// instead of every caller having to remember to sanitize before styling. +type Style struct { + lipgloss.Style +} + +// Render sanitizes strs before delegating to the wrapped lipgloss.Style. +func (s *Style) Render(strs ...string) string { + clean := make([]string, len(strs)) + for i, v := range strs { + clean[i] = Sanitize(v) + } + return s.Style.Render(clean...) +} + +// Sanitize strips terminal control bytes from s so attacker-controlled +// content (page titles, response bodies, whois records, ...) can't rewrite +// the terminal title, move the cursor, or clear the screen. \t, \n, and SGR +// color sequences (ESC [ ... m, what lipgloss emits) survive; every other +// C0/C1 control byte, DEL, OSC sequence, and non-SGR CSI sequence is dropped. +func Sanitize(s string) string { + var b strings.Builder + b.Grow(len(s)) + + for i := 0; i < len(s); { + c := s[i] + + if c == 0x1b { // ESC + if i+1 < len(s) && s[i+1] == '[' { // CSI: ESC [ params... final + j := i + 2 + for j < len(s) && s[j] >= 0x20 && s[j] <= 0x3f { + j++ + } + if j < len(s) && s[j] >= 0x40 && s[j] <= 0x7e { + final := s[j] + j++ + if final == 'm' { + b.WriteString(s[i:j]) // SGR: keep (color/bold/reset) + } + i = j + continue + } + // malformed/unterminated CSI: drop what we've consumed + i = j + continue + } + if i+1 < len(s) && s[i+1] == ']' { // OSC: ESC ] ... BEL | ESC \ + j := i + 2 + for j < len(s) { + if s[j] == 0x07 { + j++ + break + } + if s[j] == 0x1b && j+1 < len(s) && s[j+1] == '\\' { + j += 2 + break + } + j++ + } + i = j + continue + } + // bare/unrecognized ESC: drop just the byte + i++ + continue + } + + r, size := utf8.DecodeRuneInString(s[i:]) + switch { + case r == '\t' || r == '\n': + b.WriteRune(r) + case r < 0x20: + // other C0 controls (CR, BEL, backspace, ...): drop + case r == 0x7f: + // DEL: drop + case r >= 0x80 && r <= 0x9f: + // C1 controls: drop + default: + b.WriteRune(r) + } + i += size + } + + return b.String() +} + // Sink is a routable output destination: the writer chrome lands on, plus // whether interactive widgets (spinners, live progress) may animate on it. A // scan can be handed its own Sink so its chrome routes to a chosen writer. @@ -228,7 +317,7 @@ func (s *Sink) Info(format string, args ...interface{}) { if apiMode { return } - fmt.Fprintf(s.w, "%s %s\n", prefixInfo.Render("[*]"), fmt.Sprintf(format, args...)) + fmt.Fprintf(s.w, "%s %s\n", prefixInfo.Render("[*]"), Sanitize(fmt.Sprintf(format, args...))) } // Success logs a [+]-prefixed message; a no-op in API mode. @@ -236,7 +325,7 @@ func (s *Sink) Success(format string, args ...interface{}) { if apiMode { return } - fmt.Fprintf(s.w, "%s %s\n", prefixSuccess.Render("[+]"), fmt.Sprintf(format, args...)) + fmt.Fprintf(s.w, "%s %s\n", prefixSuccess.Render("[+]"), Sanitize(fmt.Sprintf(format, args...))) } // Warn logs a [!]-prefixed message; a no-op in API mode. @@ -244,7 +333,7 @@ func (s *Sink) Warn(format string, args ...interface{}) { if apiMode { return } - fmt.Fprintf(s.w, "%s %s\n", prefixWarning.Render("[!]"), fmt.Sprintf(format, args...)) + fmt.Fprintf(s.w, "%s %s\n", prefixWarning.Render("[!]"), Sanitize(fmt.Sprintf(format, args...))) } // Error logs a [-]-prefixed message; a no-op in API mode. @@ -252,7 +341,7 @@ func (s *Sink) Error(format string, args ...interface{}) { if apiMode { return } - fmt.Fprintf(s.w, "%s %s\n", prefixError.Render("[-]"), fmt.Sprintf(format, args...)) + fmt.Fprintf(s.w, "%s %s\n", prefixError.Render("[-]"), Sanitize(fmt.Sprintf(format, args...))) } func Info(format string, args ...interface{}) { DefaultSink().Info(format, args...) } @@ -308,7 +397,7 @@ func (m *ModuleLogger) Info(format string, args ...interface{}) { if apiMode { return } - msg := fmt.Sprintf(format, args...) + msg := Sanitize(fmt.Sprintf(format, args...)) fmt.Fprintf(m.sink.w, "%s %s\n", m.prefix(), msg) } @@ -317,7 +406,7 @@ func (m *ModuleLogger) Success(format string, args ...interface{}) { if apiMode { return } - msg := fmt.Sprintf(format, args...) + msg := Sanitize(fmt.Sprintf(format, args...)) fmt.Fprintf(m.sink.w, "%s %s %s\n", m.prefix(), prefixSuccess.Render("✓"), msg) } @@ -326,7 +415,7 @@ func (m *ModuleLogger) Warn(format string, args ...interface{}) { if apiMode { return } - msg := fmt.Sprintf(format, args...) + msg := Sanitize(fmt.Sprintf(format, args...)) fmt.Fprintf(m.sink.w, "%s %s %s\n", m.prefix(), prefixWarning.Render("!"), msg) } @@ -335,7 +424,7 @@ func (m *ModuleLogger) Error(format string, args ...interface{}) { if apiMode { return } - msg := fmt.Sprintf(format, args...) + msg := Sanitize(fmt.Sprintf(format, args...)) fmt.Fprintf(m.sink.w, "%s %s %s\n", m.prefix(), prefixError.Render("✗"), msg) } diff --git a/internal/output/sanitize_test.go b/internal/output/sanitize_test.go new file mode 100644 index 00000000..fa2219c2 --- /dev/null +++ b/internal/output/sanitize_test.go @@ -0,0 +1,88 @@ +package output + +import ( + "bufio" + "bytes" + "net" + "net/http" + "strings" + "testing" + "time" +) + +// regression guard: Info (and by extension Success/Warn/Error/ModuleLogger.*) +// must strip raw ANSI/OSC control bytes from the formatted message before it +// reaches the sink, even when a caller forgets to route dynamic content +// through a Style first. +func TestInfoStripsRawANSI(t *testing.T) { + var buf bytes.Buffer + old := sink + sink = &buf + defer func() { sink = old }() + + // ESC]0;PWNED BEL -> rewrites the terminal title; ESC[2J clears screen. + evil := "\x1b]0;PWNED\x07\x1b[2Jheader-value" + Info("%s", evil) + + got := buf.String() + if strings.Contains(got, "\x1b]0;PWNED\x07") { + t.Fatalf("expected OSC title sequence to be stripped, got %q", got) + } + if strings.Contains(got, "\x1b[2J") { + t.Fatalf("expected clear-screen sequence to be stripped, got %q", got) + } + if !strings.Contains(got, "header-value") { + t.Fatalf("expected legitimate content to survive, got %q", got) + } + t.Logf("CONFIRMED: Info sanitizes control bytes before printing: %q", got) +} + +// SGR color sequences (what lipgloss emits) must survive sanitization: +// Sanitize is meant to strip attacker control bytes, not legitimate styling. +func TestSanitizeKeepsSGRColor(t *testing.T) { + styled := "\x1b[1;38;5;231mhello\x1b[0m" + got := Sanitize(styled) + if got != styled { + t.Fatalf("expected SGR sequence to survive sanitize, got %q want %q", got, styled) + } +} + +// documents an existing protection: Go's net/http rejects control bytes in +// response header values at the transport layer, so headers.go's print path +// can't carry a raw ESC through a header value. +func TestHTTPHeaderRejectsESC(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + + go func() { + c, err := ln.Accept() + if err != nil { + return + } + defer c.Close() + br := bufio.NewReader(c) + for { + line, err := br.ReadString('\n') + if err != nil || line == "\r\n" { + break + } + } + resp := "HTTP/1.1 200 OK\r\n" + + "X-Evil: \x1b]0;PWNED\x07pwn\r\n" + + "Content-Length: 0\r\n\r\n" + _, _ = c.Write([]byte(resp)) + }() + + client := &http.Client{Timeout: 2 * time.Second} + r, err := client.Get("http://" + ln.Addr().String()) + if err == nil { + defer r.Body.Close() + t.Fatalf("expected Go http to reject control chars in header value, got %q", r.Header.Get("X-Evil")) + } + if !strings.Contains(err.Error(), "malformed MIME header") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/internal/scan/probe_ansi_test.go b/internal/scan/probe_ansi_test.go new file mode 100644 index 00000000..b8ac8309 --- /dev/null +++ b/internal/scan/probe_ansi_test.go @@ -0,0 +1,78 @@ +package scan + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" + + "github.com/vmfunc/sif/internal/output" +) + +// extractTitle deliberately returns the raw text unmodified: it's a +// data-extraction helper, and the JSON/log record of a probe result should be +// byte-accurate. sanitization happens downstream, at the point the title is +// printed to the terminal (see TestProbeTitleANSIStrippedFromTerminal below). +func TestExtractTitlePreservesRawBytes(t *testing.T) { + body := []byte("<html><head><title>\x1b]0;PWNED\x07\x1b[2Jinnocent") + got := extractTitle(body) + if !strings.Contains(got, "\x1b]0;PWNED\x07") || !strings.Contains(got, "\x1b[2J") { + t.Fatalf("expected extractTitle to preserve raw bytes for the result record, got %q", got) + } +} + +// regression guard: a hostile containing OSC/CSI control bytes must +// not reach the operator's terminal when Probe logs the result. probe.go +// prints output.Highlight.Render(result.Title); Highlight sanitizes its +// argument before styling it, so the escape sequences never reach the sink. +func TestProbeTitleANSIStrippedFromTerminal(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("<html><head><title>\x1b]0;PWNED\x07\x1b[2Jinnocent")) + })) + defer srv.Close() + + // output's sink is bound to os.Stdout/os.Stderr at the time SetSilent + // runs, not read fresh per-write, so swap os.Stderr *then* flip silent on + // (mirrors internal/output's own captureStdoutStderr test helper). + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + oldStderr := os.Stderr + os.Stderr = w + output.SetSilent(true) + + outCh := make(chan string, 1) + go func() { + buf, _ := io.ReadAll(r) + outCh <- string(buf) + }() + + result, err := Probe(srv.URL, 5*time.Second, "") + + output.SetSilent(false) + os.Stderr = oldStderr + w.Close() + captured := <-outCh + + if err != nil { + t.Fatalf("Probe: %v", err) + } + if !strings.Contains(result.Title, "\x1b]0;PWNED\x07") { + t.Fatalf("expected ProbeResult.Title to keep the raw bytes for the JSON record, got %q", result.Title) + } + + if strings.Contains(captured, "\x1b]0;PWNED\x07") { + t.Fatalf("expected OSC title-rewrite to be stripped from terminal output, got %q", captured) + } + if strings.Contains(captured, "\x1b[2J") { + t.Fatalf("expected clear-screen sequence to be stripped from terminal output, got %q", captured) + } + if !strings.Contains(captured, "innocent") { + t.Fatalf("expected legitimate title text to survive, got %q", captured) + } +} From 5b116394b92d5c0ca320d542a834c573c23e8f9d Mon Sep 17 00:00:00 2001 From: TBX3D <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:57:11 -0700 Subject: [PATCH 2/4] fix(scan): route whois record through the sanitized output sink Whois printed the raw record with the package-level charmbracelet logger straight to os.Stderr, bypassing apiMode/silent/lockingWriter and skipping control-byte sanitization; a whois server the target controls could inject terminal escape sequences. print through output.Info instead, which is sink-aware and sanitizes the message. --- internal/scan/whois.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/scan/whois.go b/internal/scan/whois.go index 97c427bd..fd87f25f 100644 --- a/internal/scan/whois.go +++ b/internal/scan/whois.go @@ -13,7 +13,6 @@ package scan import ( - "github.com/charmbracelet/log" "github.com/likexian/whois" "github.com/vmfunc/sif/internal/logger" "github.com/vmfunc/sif/internal/output" @@ -32,7 +31,10 @@ func Whois(url string, logdir string) { result, err := whois.Whois(sanitizedURL) if err == nil { - log.Info(result) + // route through the output sink (sanitized, apiMode/silent-aware) + // instead of the package-level charmbracelet logger, which wrote raw + // whois-response text straight to os.Stderr. + output.Info("%s", result) logger.Write(sanitizedURL, logdir, result) output.ScanComplete("WHOIS lookup", 1, "completed") } else { From 0d9b05467a81043239331cc38cf9c055234f786a Mon Sep 17 00:00:00 2001 From: TBX3D <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:57:11 -0700 Subject: [PATCH 3/4] fix(scan): point stderr module loggers at the output sink nuclei, cloudstorage, subdomaintakeover, and js/supabase each built their own charmbracelet logger with a hardcoded os.Stderr writer, bypassing apiMode/silent/lockingWriter and printing response-derived data outside the sink the rest of the tool routes through. point them at output.Writer() instead. --- internal/scan/cloudstorage.go | 3 +-- internal/scan/js/supabase.go | 4 ++-- internal/scan/nuclei.go | 2 +- internal/scan/subdomaintakeover.go | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/scan/cloudstorage.go b/internal/scan/cloudstorage.go index 617bb815..86f2f849 100644 --- a/internal/scan/cloudstorage.go +++ b/internal/scan/cloudstorage.go @@ -17,7 +17,6 @@ import ( "fmt" "io" "net/http" - "os" "strings" "time" @@ -69,7 +68,7 @@ func CloudStorage(url string, timeout time.Duration, logdir string) ([]CloudStor } } - cloudlog := log.NewWithOptions(os.Stderr, log.Options{ + cloudlog := log.NewWithOptions(output.Writer(), log.Options{ Prefix: "C3", }).With("url", url) diff --git a/internal/scan/js/supabase.go b/internal/scan/js/supabase.go index 74292766..0a89b47e 100644 --- a/internal/scan/js/supabase.go +++ b/internal/scan/js/supabase.go @@ -23,7 +23,6 @@ import ( "fmt" "io" "net/http" - "os" "regexp" "slices" "strconv" @@ -32,6 +31,7 @@ import ( "github.com/charmbracelet/log" "github.com/vmfunc/sif/internal/httpx" + "github.com/vmfunc/sif/internal/output" ) // jwtRegex matches JWT tokens in JavaScript content. @@ -175,7 +175,7 @@ func doSupabaseRequest(projectId, path, apikey string, auth *string, timeout tim } func ScanSupabase(jsContent string, jsUrl string, timeout time.Duration) ([]supabaseScanResult, error) { - supabaselog := log.NewWithOptions(os.Stderr, log.Options{ + supabaselog := log.NewWithOptions(output.Writer(), log.Options{ Prefix: "JavaScript > Supabase", }).With("url", jsUrl) diff --git a/internal/scan/nuclei.go b/internal/scan/nuclei.go index 28514417..ff919fcc 100644 --- a/internal/scan/nuclei.go +++ b/internal/scan/nuclei.go @@ -32,7 +32,7 @@ func Nuclei(url string, timeout time.Duration, threads int, logdir string) ([]ou spin := sifoutput.NewSpinner("Running nuclei templates") spin.Start() - nucleilog := log.NewWithOptions(os.Stderr, log.Options{ + nucleilog := log.NewWithOptions(sifoutput.Writer(), log.Options{ Prefix: "nuclei", }).With("url", url) diff --git a/internal/scan/subdomaintakeover.go b/internal/scan/subdomaintakeover.go index 68efda77..de9ad7a5 100644 --- a/internal/scan/subdomaintakeover.go +++ b/internal/scan/subdomaintakeover.go @@ -18,7 +18,6 @@ import ( "io" "net" "net/http" - "os" "strings" "time" @@ -86,7 +85,7 @@ func SubdomainTakeover(url string, dnsResults []string, timeout time.Duration, t } } - subdomainlog := log.NewWithOptions(os.Stderr, log.Options{ + subdomainlog := log.NewWithOptions(output.Writer(), log.Options{ Prefix: "Subdomain Takeover", }) From f25059088dd7cd24074995cc767c05e34244e733 Mon Sep 17 00:00:00 2001 From: TBX3D <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:57:11 -0700 Subject: [PATCH 4/4] fix(output): truncate progress item text on rune boundaries lastItem was sliced by byte index, so a multibyte character straddling the cutoff produced invalid UTF-8 in the progress line. truncate on rune boundaries instead. --- internal/output/progress.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/output/progress.go b/internal/output/progress.go index 955774c8..c7275533 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -16,6 +16,7 @@ import ( "fmt" "sync" "sync/atomic" + "unicode/utf8" ) // Progress bar configuration @@ -177,10 +178,12 @@ func (p *Progress) render() { } } - // Truncate item if too long + // rune-aware so a multibyte character straddling the cut point isn't + // split into invalid UTF-8. maxItemLen := 30 - if len(lastItem) > maxItemLen { - lastItem = lastItem[:maxItemLen-3] + "..." + if runeCount := utf8.RuneCountInString(lastItem); runeCount > maxItemLen { + runes := []rune(lastItem) + lastItem = string(runes[:maxItemLen-3]) + "..." } // Format: [========> ] 45% (4500/10000) /admin