Summary
gl doctor prints the node URL into three of its result rows without sanitizing it. The URL comes from --node or GITLAWB_NODE, so ANSI escapes, BEL, and a Unicode right-to-left override embedded in that value reach the operator's terminal verbatim. crates/gl/src/doctor.rs calls no sanitizer at all, while sync.rs, clone.rs, peer.rs, ipfs_cmd.rs and whoami.rs all route node-supplied strings through sanitize_node_msg.
This is the same terminal-injection class as #123 (read side) and #187 (write side), but a different input: those two are about the node's error message, this is about the node URL the user was handed. #187's inventory of 44 bail! sites does not include doctor, and doctor surfaces the value on its success path too, not only on failure.
Where
On main, crates/gl/src/doctor.rs:
:178 the node pass row, format!("{} — v{version} ({short_did}…)", args.node):197 the non-2xx row, format!("{} returned HTTP {}", args.node, resp.status()):204 the unreachable row, format!("{} unreachable: {e}", args.node)
Reproduced by execution
Built gl and ran doctor with a --node value carrying ESC, BEL, and U+202E, then read the raw bytes of the output:
$ gl doctor --node $'http://127.0.0.1:1/\x1b[31m\x07'
✗ node http://127.0.0.1:1/<ESC>[31m<BEL> unreachable: GET http://127.0.0.1:1/<ESC>[31m<BEL>/
od -c on that line shows 033, \a and 342 200 256 present unmodified. The value appears twice per row, once from args.node and once inside the reqwest error's own URL.
Why it matters
Lower severity than #187, and worth saying so plainly: the value is one the user typed or exported rather than one a node chose, so the delivery path is a copy-pasted install command, a README in an untrusted repo, or a shell profile someone else wrote, not a hostile node answering a request. What makes it worth closing anyway is that doctor is the command a confused user runs, its whole job is to be believed, and a right-to-left override can reorder a printed URL so the host reads as something else. Doctor also prints on success, so unlike the bail! sites there is no failure required to reach the sink.
Fix direction
Route all three through sanitize_node_msg (already pub(crate) in crates/gl/src/http.rs:325; it strips control characters and bidi format characters and caps at 200). Bind it once at the top of run rather than at each site, since the same value feeds every row. The {e} in the unreachable arm needs the same treatment, since the reqwest error embeds the URL again.
Two adjacent things in the same rows are node-supplied rather than caller-supplied and should go through the same helper while it is open: version and did at :178, read straight from the probed node's JSON body.
Summary
gl doctorprints the node URL into three of its result rows without sanitizing it. The URL comes from--nodeorGITLAWB_NODE, so ANSI escapes, BEL, and a Unicode right-to-left override embedded in that value reach the operator's terminal verbatim.crates/gl/src/doctor.rscalls no sanitizer at all, whilesync.rs,clone.rs,peer.rs,ipfs_cmd.rsandwhoami.rsall route node-supplied strings throughsanitize_node_msg.This is the same terminal-injection class as #123 (read side) and #187 (write side), but a different input: those two are about the node's error
message, this is about the node URL the user was handed. #187's inventory of 44bail!sites does not include doctor, and doctor surfaces the value on its success path too, not only on failure.Where
On
main,crates/gl/src/doctor.rs::178the node pass row,format!("{} — v{version} ({short_did}…)", args.node):197the non-2xx row,format!("{} returned HTTP {}", args.node, resp.status()):204the unreachable row,format!("{} unreachable: {e}", args.node)Reproduced by execution
Built
gland ran doctor with a--nodevalue carrying ESC, BEL, and U+202E, then read the raw bytes of the output:od -con that line shows033,\aand342 200 256present unmodified. The value appears twice per row, once fromargs.nodeand once inside the reqwest error's own URL.Why it matters
Lower severity than #187, and worth saying so plainly: the value is one the user typed or exported rather than one a node chose, so the delivery path is a copy-pasted install command, a README in an untrusted repo, or a shell profile someone else wrote, not a hostile node answering a request. What makes it worth closing anyway is that doctor is the command a confused user runs, its whole job is to be believed, and a right-to-left override can reorder a printed URL so the host reads as something else. Doctor also prints on success, so unlike the
bail!sites there is no failure required to reach the sink.Fix direction
Route all three through
sanitize_node_msg(alreadypub(crate)incrates/gl/src/http.rs:325; it strips control characters and bidi format characters and caps at 200). Bind it once at the top ofrunrather than at each site, since the same value feeds every row. The{e}in the unreachable arm needs the same treatment, since the reqwest error embeds the URL again.Two adjacent things in the same rows are node-supplied rather than caller-supplied and should go through the same helper while it is open:
versionanddidat:178, read straight from the probed node's JSON body.