Sanitize control characters in contract spec display output.#2433
Merged
Sanitize control characters in contract spec display output.#2433
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens Stellar CLI contract-spec rendering by stripping control characters from spec-derived strings at display/help-text boundaries to prevent terminal escape-sequence injection via malicious contracts.
Changes:
- Re-exported and applied a shared
sanitizehelper to strip control characters from spec names/docs before printing or registering with clap help. - Added a sanitized-name fallback lookup in
get_function_specto keep functions with control characters addressable via their sanitized clap subcommand. - Added regression tests + a wasm fixture containing ANSI escape sequences to validate help/spec output is free of unexpected control characters.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/soroban-cli/src/commands/contract/arg_parsing.rs | Sanitizes function names/docs used in clap command/help generation; adds lookup fallback and a help-rendering regression test. |
| cmd/crates/soroban-spec-tools/src/contract.rs | Sanitizes spec display output (meta entries, function names/docs, UDT names/docs) and defines the shared sanitize helper. |
| cmd/crates/soroban-spec-tools/src/lib.rs | Re-exports contract::sanitize for use by CLI code. |
| cmd/crates/soroban-spec-tools/tests/contract_sanitize.rs | Adds a test ensuring Spec display output contains no unexpected control characters. |
| cmd/crates/soroban-spec-tools/tests/fixtures/control_characters.wasm | Adds a malicious fixture embedding ANSI escape sequences in spec fields for regression coverage. |
8f2f96c to
b5e2e76
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cmd/soroban-cli/src/commands/contract/arg_parsing.rs:358
sanitize()preserves\n, so leakingsanitize(name)into clap’s command name can produce subcommand names containing newlines (untypable from a shell) and can still inject extra lines into help output. It can also produce an empty string if the name is entirely control characters, and it doesn’t guard against multiple distinct function names collapsing to the same sanitized clap name. Consider using a stricter “identifier sanitizer” for clap names (e.g., reject/strip all whitespace/control incl. newlines, enforce non-empty) and detect collisions early with a clear error.
let name: &'static str = Box::leak(sanitize(name).into_boxed_str());
let mut cmd = clap::Command::new(name)
.no_binary_name(true)
.term_width(300)
.max_term_width(300);
leighmcculloch
approved these changes
Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Contract spec fields (function names, docstrings, UDT names, metadata key/val) are now stripped of ASCII control characters before being written to the terminal or registered as clap help text. This prevents a malicious contract from embedding ANSI escape sequences that could overwrite terminal output with forged content.
Affected paths:
stellar contract invoke --help(clap subcommand names and help text)stellar contract info(Spec Display impl and text output)stellar contract inspect(deprecated)Tests added using a fixture wasm that embeds ANSI escape sequences in function names and docstrings.
Why
Known limitations
N/A