Skip to content

tracing: %-formatted fields put client control bytes on stderr unescaped #266

Description

@plusky

What

A %-formatted tracing field reaches stderr with its control bytes intact. tracing-subscriber 0.3.23's DefaultVisitor runs EscapeGuard (ESC, BEL, BS, FF, DEL, C1 \x80\x9f\x1b-style escapes) over the message field and record_error only; a ? field is Debug-escaped by str's own impl; a % field is record_debug(&format_args!("{}", v)), whose Debug is its Display, written verbatim.

Measured with a scratch binary on the locked tracing-subscriber (with_ansi(false), the shipped configuration):

INFO esc: display field query=a^[[2Jb^GcM-BM-^[d      <- %Capped(q): raw ESC, BEL, U+009B
INFO esc: debug field query="a\u{1b}[2Jb\u{7}c\u{9b}d"  <- ?q
INFO esc: message a\x1b[2Jb\x07c\u{9b}d                <- message: EscapeGuard
WARN esc: display error error=err^[[2Jx                <- error = %e

Every client string bugwarden puts in a field is %: query, status, include_fields (server.rs:2911), product/component (:3062, :3138), file_name (:3175), status (:3273), assignee (:3319), cc_email (:3619), client_requested (:4245) — #240 kept the % the sites had (%p.query at 924793b), so this predates it — and the error = %e sites carry Bugzilla's message the same way (#261). A query of \x1b[2J\x1b[H clears the operator's terminal; \x1b]0;…\x07 retitles it; the DESIGN.md audit-boundary rule that a client string is never a control channel to the operator holds for the audit record (JSON-escaped) and for message but not for these fields. Over OTLP the bytes are inside a protobuf string, so the collector side is whatever the viewer does with them — the same as today for any field.

Direction

Either switch the client-string sites to ?Capped(..) (Debug escapes, and Capped already implements it — the line shape changes from query=foo to query="foo", which binary_tracing_caps.rs's logged_field needles must follow) or, if #260 lands a sink-side FormatFields, escape there for every field regardless of how the site spelled it — EscapeGuard is pub(super), so that is a copy of its five-byte match, not a reuse. The second covers rmcp's %id and client_requested lines too. Pin in binary_tracing_caps.rs with an ESC-carrying query and assert no raw \x1b on stderr. Found reading tracing-subscriber's visitor while designing #260.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions