ansiescape: cover all control-sequence introducers, not just CSI/OSC#1980
Open
WatchTree-19 wants to merge 1 commit into
Open
ansiescape: cover all control-sequence introducers, not just CSI/OSC#1980WatchTree-19 wants to merge 1 commit into
WatchTree-19 wants to merge 1 commit into
Conversation
The ansiescape detectors built their needles from a hardcoded
suffixes = {[, ]}, so only CSI and OSC were recognised. ESC c (RIS)
and the DCS/SOS/PM/APC string controls -- plus their C1 single-byte forms
0x90/0x98/0x9e/0x9f -- scored 0.0 in both Raw and Escaped, leaving the
module narrower than the attack class its docstring describes. APC in
particular is live in the wild via the kitty graphics protocol.
Widening the suffix list naively introduces false positives, so the
introducers are split in two:
- INTRODUCERS ([, ], ^, _) are punctuation and safe to cross with every
command spelling.
- ALPHA_INTRODUCERS (c, P, X) are not. StringDetector matches
case-insensitively (case_sensitive defaults to False), so crossing them
with the shell-style \e spelling yields the needles \ec, \ep and \ex,
which match benign text such as \echo, \epsilon and \export. For the
Escaped detector these are instead pinned against the unambiguous
command spellings, derived from ESCAPED_COMMANDS so they track any
future additions. RAW_COMMANDS are control bytes rather than text and
carry no such risk, so Raw takes the full set.
RIS is covered for detection and appears in ESCAPED_PAYLOADS as literal
text, but is deliberately kept out of LIVE_PAYLOAD_TOKENS: unlike every
other entry there it is a complete two-byte terminal reset rather than an
inert introducer, and per the module note these payloads land in both
garak and remote logs.
ST (0x9c), charset designation and DECALN remain out of scope.
Tests pin each newly covered introducer in both raw and escaped form,
assert the two detectors do not cross-fire, and keep benign text --
including the \e-prefixed words above -- at 0.0. The pre-existing
LIVE/ESCAPED payload separation is unchanged.
Signed-off-by: WatchTree-19 <119982314+WatchTree-19@users.noreply.github.com>
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 #1977. Supersedes #1978 — that PR was closed after I force-pushed a malformed commit to its branch (an
--amendinside a shallow clone produced a parentless commit that appeared to touch 811 files). The branch is now correct: one commit, three files, on top ofmain.The gap
Both
ansiescapedetectors built their needles from a hardcodedsuffixes = {"[", "]"}, so only CSI and OSC were recognised.ESC c(RIS) and the DCS/SOS/PM/APC string controls — plus their C1 single-byte forms0x90/0x98/0x9e/0x9f— scored0.0in bothRawandEscaped. APC is not theoretical: it is live in the wild via the kitty graphics protocol.As #1977 notes, this is a coverage boundary rather than a live false negative — all seven existing
LIVE_PAYLOADSstill score1.0today. The nine previously-uncovered sequences (RIS, DCS, SOS, PM, APC and the four C1 forms) go from0.0to1.0.Why it isn't just a wider suffix list
Widening the list naively introduces false positives, which is the substantive part of this change.
StringDetectormatches case-insensitively (case_sensitivedefaults toFalse), so crossing the alphabetic introducers with the shell-style\espelling produces the needles\ec,\epand\ex— which match benign text like\echo,\epsilonand\export. I hit this via a failing test rather than by inspection.So the introducers are split:
INTRODUCERS([,],^,_) — punctuation, safe to cross with every command spelling.ALPHA_INTRODUCERS(c,P,X) — pinned for theEscapeddetector against the unambiguous command spellings only, derived fromESCAPED_COMMANDSso they track future additions automatically.RAW_COMMANDSare control bytes rather than text and carry no such risk, soRawtakes the full set.RIS
RIS is covered for detection and included in
ESCAPED_PAYLOADSas literal text, but deliberately kept out ofLIVE_PAYLOAD_TOKENS. Unlike every other entry there — all inert, incomplete introducers —ESC cis a complete two-byte terminal reset, and the module note is explicit that these payloads land in both garak and remote logs. Happy to add it if you'd rather have the coverage.ST(0x9c), charset designation and DECALN remain out of scope per the issue.Verification
tests/detectors/test_detectors_ansiescape.pyandtests/probes/test_probes_ansiescape.py— 11 passed.\e-prefixed words above) at0.0.LIVE_PAYLOADS→Raw-only /ESCAPED_PAYLOADS→Escaped-only separation still holds with 0 violations, including when simulated against the widenedESCAPED_COMMANDSfrom ansiescape: detect alternative escaped ESC encodings (2-digit octal, curly/8-digit unicode) #1929.blackclean, DCO signed off.Note on overlap with #1929
This touches
ansi.pyandtest_detectors_ansiescape.py, as does my open #1929. The two are orthogonal axes — #1929 widens the ESC encoding set (ESCAPED_COMMANDS), this widens the introducer set — and they compose, since needles are the cross-product. I checked that combination explicitly for false positives (clean, above). Whichever lands first, I'll rebase the other.