Strip terminal escape sequences from the Command column - #951
Open
carfeii wants to merge 1 commit into
Open
Conversation
Command::add read each process's command line and only replaced newline and tab characters before displaying it, with no handling of other control characters or ANSI/terminal escape sequences. Since a process's command line (including argv[0]) is fully controlled by whichever user started it, and /proc/<pid>/cmdline is world-readable on Linux regardless of process ownership, any local unprivileged user could plant a process whose command line contains a raw escape sequence, which would then be printed unmodified to any other user's terminal when they run `procs`, a routine action that displays every visible process. Add util::sanitize_display, which replaces control characters (including ESC) with the Unicode replacement character, and apply it to the final command-line string in all four platform variants of Command::add (Linux/Android, macOS, Windows, FreeBSD). See dalance#950.
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.
Fixes #950.
Summary
Command::add(src/columns/command.rs) read each process's command lineand only replaced newline and tab characters before displaying it, with
no handling of other control characters or ANSI/terminal escape
sequences. Since a process's command line (including
argv[0]) is fullycontrolled by whichever user started it, and
/proc/<pid>/cmdlineisworld-readable on Linux regardless of process ownership, any local
unprivileged user could plant a process whose command line contains a
raw escape sequence. That sequence would then be printed unmodified to
any other user's terminal the moment they ran
procs, since displayingevery visible process is the tool's whole purpose, not an action taken
with awareness of viewing untrusted content the way opening a specific
file is.
Fix
Adds
util::sanitize_display, which replaces control characters(including the
ESCbyte,0x1b) with the Unicode replacementcharacter, and applies it to the final command-line string in all four
platform variants of
Command::add(Linux/Android, macOS, Windows,FreeBSD). The existing newline/tab-to-space replacement is kept as-is;
this is an additional pass on top of it.
Testing
cargo test --releasepasses in full (15 passed, up from 13 with the2 new tests, 0 failed).
sanitize_display_strips_escape_sequencesandsanitize_display_preserves_normal_texttosrc/util.rs.detached process (
subprocess.Popenwith a craftedargv[0]containinga raw OSC 52 escape sequence,
executable=pointing atsleepso theactual command run was harmless), ran
procsagainst it, and confirmedthe exact injected byte sequence appeared unmodified in
procs's rawoutput. Repeated against this branch and confirmed the
ESC/BELbytes are replaced with the Unicode replacement character instead.