wrapper-guard: recognize program names after quote removal, in one place
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Round-four remediation of both blockers gate-ultron-01 raised ond99ff57e. Measured against that head before anything was touched; all seven returned rc=0, and each executes the program the check exists to recognize: "/usr/bin/curl" --config /tmp/w.cfg -> allowed './curl' --config /tmp/w.cfg -> allowed $(which curl) --config /tmp/w.cfg -> allowed `which curl` --config /tmp/w.cfg -> allowed /usr/bin/gh api -X POST repos/a/b/issues … -> allowed ./gh api -X POST repos/a/b/issues … -> allowed /usr/local/bin/tea api -X POST repos/a/b/… … -> allowed This is the third appearance of one defect, and the shape is worth stating plainly because the first two repairs each fixed an INSTANCE and left the class: the check matched the bare word, then it matched the unquoted basename. Both were models of one TEXTUAL PRESENTATION of a shell word rather than of the word, so the first repair was defeated by an absolute path and the second by two quote characters. Recognizing a name is either done after quote removal or it is caller-name parsing wearing a longer regex. The second blocker is the same defect sitting untouched in the API SCOPE gate the whole time, while the curl arm was repaired twice beside it. That one is worse than it looks: the scope gate decides whether write detection runs AT ALL, so failing to admit `/usr/bin/gh api -X POST` is not a missed match, it is an allow. No URL marker rescued those commands either — provider CLI endpoints are spelled `repos/…` with no leading slash, so `/repos/` never matched them. Fix, and the reason it is one fix rather than two: - $CMD_NAMES — a second reading of the same command with quote and substitution punctuation turned into whitespace. Names are read from it. - $NAME_PREFIX — the one place the shape of a program name is written down. Both callers use it, so the next fix to this class lands in a single location instead of whichever arm review happened to probe. That is the actual lesson of finding this defect twice in one file. The prefix still must end at a slash. `mycurl` and `curl-wrapper` are different programs and blocking them is the over-block that gets a guard routed around instead of repaired; both remain negative fixtures, and `mygh` and an absolute-path READ join them. The cost is the one this file already chose and documented for the payload check: quoting an example does not exempt it, so writing one of these commands inside quotes on a Bash line is refused too. Applying that rule to the name arms makes the file coherent — the alternative is a guard where the payload arm treats quotes as text and the name arms treat them as armour. Still open, stated rather than left to be found: a name absent from the text — assembled from variables, or reached through a wrapper script that execs the program — is invisible here. That is a limit of inspecting a command string, not something a pattern closes. Controls: the 7 positive fixtures FAIL atd99ff57eand pass here; the 4 negative fixtures pass at BOTH heads, so they measure over-blocking rather than decorate the diff. Suite 157/157.
This commit is contained in:
@@ -295,6 +295,25 @@ FIXTURES="$TMP/fixtures.tsv"
|
|||||||
printf '0\t{"tool_input":{"command":"mycurl --config /tmp/provider-write.cfg"}}\tmycurl is not curl, and over-blocking is its own failure\n'
|
printf '0\t{"tool_input":{"command":"mycurl --config /tmp/provider-write.cfg"}}\tmycurl is not curl, and over-blocking is its own failure\n'
|
||||||
printf '0\t{"tool_input":{"command":"/opt/x/curl-wrapper --config /tmp/provider-write.cfg"}}\tnor is curl-wrapper, whose name only starts the same way\n'
|
printf '0\t{"tool_input":{"command":"/opt/x/curl-wrapper --config /tmp/provider-write.cfg"}}\tnor is curl-wrapper, whose name only starts the same way\n'
|
||||||
|
|
||||||
|
# And the same name once it is punctuated. The basename repair above fixed the
|
||||||
|
# UNQUOTED path spelling and nothing else, so two quote characters restored the
|
||||||
|
# bypass it had just closed: the check was still modelling one presentation of
|
||||||
|
# a shell word instead of the word. Every one of these executes the real curl.
|
||||||
|
printf '2\t{"tool_input":{"command":"\\"/usr/bin/curl\\" --config /tmp/provider-write.cfg"}}\tquoting a path does not make it a different program\t--config/-K\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"'"'"'./curl'"'"' --config /tmp/provider-write.cfg"}}\tnor does quoting a relative one\t--config/-K\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"$(which curl) --config /tmp/provider-write.cfg"}}\tthe name is in the text even when a substitution supplies the path\t--config/-K\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"`which curl` --config /tmp/provider-write.cfg"}}\tand in the older spelling of the same substitution\t--config/-K\n'
|
||||||
|
|
||||||
|
# The provider-CLI SCOPE gate had the identical defect, untouched while the
|
||||||
|
# curl arm was repaired twice. It decides whether write detection runs at all,
|
||||||
|
# so failing to admit these is indistinguishable from allowing them — and no
|
||||||
|
# URL marker rescues them, because provider CLI paths carry no leading slash.
|
||||||
|
printf '2\t{"tool_input":{"command":"/usr/bin/gh api -X POST repos/a/b/issues -f title=x"}}\tan absolute path to a provider CLI is still a provider CLI\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"./gh api -X POST repos/a/b/issues -f title=x"}}\tand a relative one still is too\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"/usr/local/bin/tea api -X POST repos/a/b/issues -f title=x"}}\tthe same is true of every CLI the gate names, not just the first\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"mygh api -X POST repos/a/b/issues -f title=x"}}\tmygh is not gh, and the scope gate must not over-admit either\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"/usr/bin/gh api repos/a/b/issues"}}\ta read through an absolute path is still a read\n'
|
||||||
|
|
||||||
# Percent-encoded endpoints. Not hypothetical: /issues/1174 and /iss%%75es/1174
|
# Percent-encoded endpoints. Not hypothetical: /issues/1174 and /iss%%75es/1174
|
||||||
# both returned HTTP 200 with the same object from the live forge, so the
|
# both returned HTTP 200 with the same object from the live forge, so the
|
||||||
# encoded spelling IS the wrapped endpoint and the literal comparison below it
|
# encoded spelling IS the wrapped endpoint and the literal comparison below it
|
||||||
|
|||||||
@@ -48,6 +48,47 @@ CMD="$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null |
|
|||||||
# joined form, because that is the command.
|
# joined form, because that is the command.
|
||||||
CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
||||||
|
|
||||||
|
# A second reading of the SAME command, used only by the two checks below that
|
||||||
|
# have to recognize a program by name. Quote characters and the punctuation of
|
||||||
|
# command substitution become whitespace, so a name that the shell will resolve
|
||||||
|
# is a word here regardless of how it was dressed:
|
||||||
|
#
|
||||||
|
# "/usr/bin/curl" --config /tmp/req quoted absolute path
|
||||||
|
# './curl' --config /tmp/req quoted relative path
|
||||||
|
# $(which curl) --config /tmp/req command substitution
|
||||||
|
# `which curl` --config /tmp/req the older spelling of the same thing
|
||||||
|
# /usr/bin/gh api -X POST repos/a/b/... absolute path to a provider CLI
|
||||||
|
#
|
||||||
|
# All five executed the real program and all five returned ALLOW from the two
|
||||||
|
# name checks, at this file's previous head and the one before it. The first
|
||||||
|
# repair of this class matched curl as a basename and stopped there, which fixed
|
||||||
|
# the unquoted spelling only: the check still modelled ONE TEXTUAL PRESENTATION
|
||||||
|
# of a shell word rather than the word itself, so adding two quote characters
|
||||||
|
# restored the bypass, and the same defect sat untouched in the provider-CLI arm
|
||||||
|
# the whole time. Recognizing a name is either done after quote removal or it is
|
||||||
|
# caller-name parsing wearing a longer regex.
|
||||||
|
#
|
||||||
|
# This does NOT try to be a shell. It cannot see a name that is absent from the
|
||||||
|
# text — assembled from variables, or reached through a wrapper script that
|
||||||
|
# execs the program — and those remain stated limits of inspecting a command
|
||||||
|
# string, not defects a pattern closes. What it removes is the class where the
|
||||||
|
# name IS present and merely punctuated.
|
||||||
|
#
|
||||||
|
# The cost is the one this file already chose and documented for the payload
|
||||||
|
# check further down: quoting an example no longer exempts it, so writing one of
|
||||||
|
# these commands inside quotes on a Bash line is refused too. Applying that same
|
||||||
|
# rule here keeps the file coherent — the alternative is a guard where the
|
||||||
|
# payload arm treats quotes as text and the name arms treat them as armour.
|
||||||
|
CMD_NAMES="$(printf '%s' "$CMD" | tr '"'"'"'\`()' ' ')"
|
||||||
|
CMD_NAMES="$(printf '%s' "$CMD_NAMES" | sed 's/\$/ /g')"
|
||||||
|
|
||||||
|
# The one place the shape of a program NAME is written down. Both callers below
|
||||||
|
# use it, so the next fix to this class lands in a single location instead of
|
||||||
|
# being applied to whichever arm review happened to probe. The prefix must end
|
||||||
|
# at a slash: `mycurl` and `curl-wrapper` are different programs, and blocking
|
||||||
|
# them is the over-block that gets a guard routed around instead of repaired.
|
||||||
|
NAME_PREFIX='(^|[[:space:]|;&])([^[:space:]|;&]*/)?'
|
||||||
|
|
||||||
# Honour the override only where a shell would actually TREAT it as one: the
|
# Honour the override only where a shell would actually TREAT it as one: the
|
||||||
# environment-assignment run at the head of the command, or this process's own
|
# environment-assignment run at the head of the command, or this process's own
|
||||||
# environment. The first version asked whether the token appeared ANYWHERE in the
|
# environment. The first version asked whether the token appeared ANYWHERE in the
|
||||||
@@ -291,17 +332,15 @@ fi
|
|||||||
# deleting one character. Scoped to curl so that `eslint --config .eslintrc.json`
|
# deleting one character. Scoped to curl so that `eslint --config .eslintrc.json`
|
||||||
# and every other tool with a --config flag are untouched.
|
# and every other tool with a --config flag are untouched.
|
||||||
#
|
#
|
||||||
# curl is matched as a BASENAME, not as a bare word: `/usr/bin/curl`, `./curl`
|
# curl is recognized as a NAME, through $CMD_NAMES and $NAME_PREFIX — see the
|
||||||
# and `env /usr/bin/curl` are ordinary spellings of the same invocation, and the
|
# comment on those at the top of the file for why matching the bare word, and
|
||||||
# first version of this check saw none of them. Recognizing only the unqualified
|
# then matching the unquoted basename, were both the same mistake at different
|
||||||
# name is caller-name parsing, which is the failure class this file removed
|
# depths. A wrapper script that execs curl on the operator's behalf is still
|
||||||
# elsewhere and which came straight back in with this control.
|
# invisible here, because neither the name nor the request appears in the
|
||||||
#
|
# command text at all. That is a limit of inspecting a command string rather
|
||||||
# A wrapper script that execs curl on the operator's behalf is still invisible
|
# than a defect this pattern can close, and it is stated rather than left for
|
||||||
# here, because neither the name nor the request appears in the command text.
|
# the next reader to find.
|
||||||
# That is a real limit of inspecting a command string rather than a defect this
|
if printf '%s' "$CMD_NAMES" | grep -Eq "${NAME_PREFIX}curl([[:space:]]|$)" \
|
||||||
# regex can close, and it is stated rather than left for the next reader to find.
|
|
||||||
if printf '%s' "$CMD" | grep -Eq '(^|[[:space:]|;&(])([^[:space:]|;&()]*/)?curl([[:space:]]|$)' \
|
|
||||||
&& printf '%s' "$CMD" | grep -Eq -- '(^|[[:space:]])(-[A-Za-z]*K([[:space:]=]|$|[^[:space:]])|--config([[:space:]=]|$))'; then
|
&& printf '%s' "$CMD" | grep -Eq -- '(^|[[:space:]])(-[A-Za-z]*K([[:space:]=]|$|[^[:space:]])|--config([[:space:]=]|$))'; then
|
||||||
cat <<EOF
|
cat <<EOF
|
||||||
BLOCKED: curl invocation whose request is supplied from a --config/-K file.
|
BLOCKED: curl invocation whose request is supplied from a --config/-K file.
|
||||||
@@ -321,9 +360,24 @@ EOF
|
|||||||
exit 2
|
exit 2
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# The scope gate comes in two halves because its two triggers are different
|
||||||
|
# kinds of thing, and collapsing them into one regex over one string is what
|
||||||
|
# hid the second instance of the caller-name defect for three review rounds.
|
||||||
|
#
|
||||||
|
# The URL markers are TEXT: a scheme, a version segment, a `/repos/` path. They
|
||||||
|
# are read from the command as written.
|
||||||
API_SHAPED='https?://|/api/v[0-9]|/repos/'
|
API_SHAPED='https?://|/api/v[0-9]|/repos/'
|
||||||
API_SHAPED="$API_SHAPED"'|(^|[[:space:]|;&(])(gh|tea|glab|hub)[[:space:]]+api([[:space:]]|$)'
|
#
|
||||||
if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then
|
# The provider-CLI marker is a NAME, so it is read the way names are read here —
|
||||||
|
# through $CMD_NAMES, with $NAME_PREFIX. Asking for a bare `gh api` let every
|
||||||
|
# path spelling through the gate that decides whether write detection runs AT
|
||||||
|
# ALL, so `/usr/bin/gh api -X POST repos/a/b/issues -f title=x` was never even
|
||||||
|
# considered. No URL marker rescued it: provider CLI endpoints are spelled
|
||||||
|
# `repos/...` with no leading slash, so `/repos/` does not match them either.
|
||||||
|
# A scope gate that fails to admit is indistinguishable from an allow.
|
||||||
|
PROVIDER_CLI="${NAME_PREFIX}(gh|tea|glab|hub)[[:space:]]+api([[:space:]]|$)"
|
||||||
|
if printf '%s' "$CMD" | grep -Eq "$API_SHAPED" \
|
||||||
|
|| printf '%s' "$CMD_NAMES" | grep -Eq "$PROVIDER_CLI"; then
|
||||||
|
|
||||||
# Write detection, now client-agnostic. Every spelling curl accepts, because
|
# Write detection, now client-agnostic. Every spelling curl accepts, because
|
||||||
# the guard is defeated by the one spelling it does not know: `-d@body` (no
|
# the guard is defeated by the one spelling it does not know: `-d@body` (no
|
||||||
|
|||||||
Reference in New Issue
Block a user