fix(output): sanitize control bytes and truncate rune-safe before terminal print#336
fix(output): sanitize control bytes and truncate rune-safe before terminal print#336TBX3D wants to merge 4 commits into
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #336 +/- ##
=======================================
Coverage ? 64.93%
=======================================
Files ? 88
Lines ? 7908
Branches ? 0
=======================================
Hits ? 5135
Misses ? 2379
Partials ? 394 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
scan
pr summary10 files changed (+311 -27)
|
86cb731 to
0440fd1
Compare
vmfunc
left a comment
vmfunc
left a comment
There was a problem hiding this comment.
the split is right: sanitize at the print boundary, leave extractTitle / ProbeResult.Title / logger.Write byte-accurate so the json + file record stay true and only the terminal gets scrubbed. probe_ansi_test pins exactly that.
sanitize keeping only m-final CSI is correct, sgr color survives, cursor/clear/osc dropped, and it's a linear scan so no redos. output.Info is printf-style so output.Info("%s", result) not output.Info(result) in whois was the right call, keeps a % in a whois record from being eaten.
one nit: Style.Render is a pointer receiver. every call site today is an addressable package var so it's fine, but a non-addressable Style (map elem, or one assigned into an interface) would bind the promoted lipgloss Render and skip sanitize silently. make it a value receiver, nothing here mutates and it closes that off.
conflicting with main, rebase and it's in.
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.
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.
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.
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.
a5714c8 to
f250590
Compare
|
rebased. output.go was restructured around the |
3 participants
several code paths wrote to stderr/stdout directly instead of through the
output sink: module loggers, the whois record dump, and a couple of scan
status lines. under -concurrency>1, only the sink is lock-wrapped, so a
raw fmt.Println/log write could interleave or tear with everything else.
those now route through output.Info/Error.
Sanitize also strips terminal control bytes (OSC, non-SGR CSI, C0/C1,
DEL) from attacker-controlled content before it reaches the terminal,
while keeping \t/\n and SGR color codes intact so styling still works.
progress-item truncation is now rune-aware so a multibyte character
straddling the cut point isn't split into invalid UTF-8.