fix(git): fail closed on forbidden control chars, fix float exponent + '?' over-rejection (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful

Round-9 auditor blockers, both fixed at root cause in the PyYAML-absent
line-parser fallback of get_gitea_token_for_login:

1. Control-character fail-open (dangerous): PyYAML 6.0.3's Reader rejects
   the WHOLE document (ReaderError) if a forbidden C0/DEL control byte
   {0x00-0x08, 0x0B, 0x0C, 0x0E-0x1F, 0x7F} appears ANYWHERE in the raw
   stream -- plain scalar, inside quotes, or a comment -- regardless of
   position, verified empirically against the real installed PyYAML 6.0.3.
   The fallback previously only guarded tabs and emitted the login token
   from such documents. Added a whole-document _FORBIDDEN_CONTROL scan on
   the raw text (before splitlines(), which itself splits on some of the
   same bytes) so the fallback fails closed identically to PyYAML.

2. Unsigned-exponent float over-rejection: PyYAML's implicit float
   resolver requires an EXPLICIT sign on the exponent; unsigned-exponent
   spellings (1.0e10, +1.0e10, -1.0e10, 1.0E10, .5e10, 4.e8) are PyYAML
   STRINGS, not floats. The fallback's _IMPLICIT_FLOAT pattern allowed an
   optional sign, misclassifying these as floats and dropping the token.
   Tightened the regex to match PyYAML's resolver exactly.

Also folds in a residual over-rejection found via this round's wide
differential fuzz (1767 cases, 0 fail-opens / 0 over-rejections after the
fix): a plain scalar starting with "?" not followed by whitespace (e.g.
"?x") is a valid PyYAML string, but the fallback's blanket-reject set
treated every leading "?" as illegal. Narrowed to match the existing
space-aware handling already used for "-" and ":".

Adds regression fixtures to test-gitea-login-resolution.sh covering all
three fixes under both PyYAML-present and forced-ImportError-absent runs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Hermes Agent
2026-07-22 00:39:37 -05:00
parent 6053e1ee4c
commit acf7955f87
2 changed files with 171 additions and 12 deletions

View File

@@ -640,9 +640,14 @@ _IMPLICIT_INT = re.compile(
r"|[-+]?0x[0-9a-fA-F_]+"
r"|[-+]?[1-9][0-9_]*(?::[0-5]?[0-9])+)$"
)
# NOTE: the exponent group requires an EXPLICIT sign ([eE][-+][0-9]+), matching
# PyYAML 6.0.3's resolver.py float regex exactly. An unsigned exponent (e.g.
# "1.0e10", ".5e10", "4.e8") is NOT matched by PyYAML's implicit float resolver
# -- PyYAML resolves those as plain strings -- so this pattern must not match
# them either, or the fallback over-rejects a token PyYAML would emit verbatim.
_IMPLICIT_FLOAT = re.compile(
r"^(?:[-+]?(?:[0-9][0-9_]*)\.[0-9_]*(?:[eE][-+]?[0-9]+)?"
r"|\.[0-9][0-9_]*(?:[eE][-+]?[0-9]+)?"
r"^(?:[-+]?(?:[0-9][0-9_]*)\.[0-9_]*(?:[eE][-+][0-9]+)?"
r"|\.[0-9][0-9_]*(?:[eE][-+][0-9]+)?"
r"|[-+]?[0-9][0-9_]*(?::[0-5]?[0-9])+\.[0-9_]*"
r"|[-+]?\.(?:inf|Inf|INF)"
r"|\.(?:nan|NaN|NAN))$"
@@ -660,9 +665,10 @@ def _implicit_nonstring(text):
# True when an UNQUOTED plain scalar would be resolved by PyYAML's SafeLoader
# to a non-string type (null/bool/int/float/timestamp). Fuzzed against real
# PyYAML 6.0.3: it never returns False where PyYAML types the scalar as a
# non-string (i.e. never fail-open), and is at worst MORE conservative on a
# couple of degenerate float spellings (e.g. "4.e8") that PyYAML keeps as a
# string -- the safe direction for this credential-selecting fallback.
# non-string (i.e. never fail-open), and (post exponent-sign fix) no longer
# over-rejects unsigned-exponent spellings like "4.e8" or "1.0e10" -- those
# match PyYAML's own implicit float resolver exactly (explicit sign only),
# so PyYAML keeps them as strings and this function now agrees.
return bool(
_IMPLICIT_NULL.match(text)
or _IMPLICIT_BOOL.match(text)
@@ -721,11 +727,10 @@ def _int_constructible(text):
def _float_constructible(text):
# Replicate PyYAML SafeConstructor.construct_yaml_float. Retained for the
# WHOLE-document invariant even though PyYAML's float-tagged set is always
# float()-constructible: the fallback's float RESOLVER pattern is a strict
# superset of PyYAML's (it also matches unsigned-exponent spellings PyYAML
# keeps as strings), and every such extra is likewise constructible, so this
# never fails closed where PyYAML would emit a token.
# WHOLE-document invariant: _IMPLICIT_FLOAT now matches PyYAML's resolver
# pattern exactly (explicit-sign exponent only), and every scalar it tags
# is float()-constructible, so this never fails closed where PyYAML would
# emit a token.
value = text.replace("_", "").lower()
if value[:1] in ("+", "-"):
value = value[1:]
@@ -804,6 +809,22 @@ def _constructible(text):
return True
# PyYAML's Reader.check_printable scans the ENTIRE raw input stream (not just
# scalar contents, and NOT scoped by quoting) for bytes outside its printable
# set and raises ReaderError -- a whole-document reject -- the instant one is
# found, no matter where it sits (an unrelated field, a comment, inside or
# outside quotes). Empirically verified against real PyYAML 6.0.3: EVERY C0
# control byte below plus DEL (0x7F) rejects in plain scalars, inside
# double-quoted scalars, and inside single-quoted scalars alike; only
# TAB(0x09), LF(0x0A), and CR(0x0D) are accepted among the C0 range (LF/CR are
# line separators, handled elsewhere; TAB has its own narrower, position-aware
# guard above since PyYAML accepts it in some contexts). This set therefore
# excludes 0x09/0x0A/0x0D and matches the forbidden C0 set {0x00-0x08, 0x0B,
# 0x0C, 0x0E-0x1F} plus DEL (0x7F).
_FORBIDDEN_CONTROL = re.compile(
"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]"
)
# Sentinel: "outside the supported subset -> fail closed". Distinct from a
# genuine null (None), which is a valid resolved value.
_FAIL = object()
@@ -865,8 +886,15 @@ def _scalar(raw):
value = value.strip(" ") # spaces only; a tab must survive to fail closed
if not value:
return None
if value[0] in _FLOW or value[0] in ("|", ">", "?", "@", "`", '"', "'", "%", ","):
if value[0] in _FLOW or value[0] in ("|", ">", "@", "`", '"', "'", "%", ","):
return _FAIL # flow / block-scalar / reserved / directive / anchor / quote
# NOTE: "?" is deliberately NOT in the blanket-reject set above. Empirically
# verified against real PyYAML 6.0.3: "?x" (indicator immediately followed by
# a non-space) is a plain scalar string "?x" like "-x" and ":x" are -- only
# "?" alone or "? " (followed by space/EOL) opens a complex mapping key and
# is illegal in a value position. Blanket-rejecting every leading "?" would
# over-reject a token PyYAML accepts verbatim; the narrower check below
# (mirroring "-" and ":") handles exactly the illegal bare-indicator forms.
if value[0] in ("-", "?", ":") and (len(value) == 1 or value[1] in (" ", "\t")):
# A bare block indicator, not a plain scalar: '-'/'- ' opens a sequence
# entry, '?'/'? ' a complex mapping key, ':'/': ' a mapping value -- all
@@ -1151,7 +1179,21 @@ def _token_via_lines():
# whose `name` equals the request -> host/port-bound token), so the fallback
# can only ever be MORE conservative than PyYAML, never less.
with open(config_path, encoding="utf-8") as handle:
lines = handle.read().splitlines()
raw_text = handle.read()
# Whole-document, position-independent reject: PyYAML's Reader rejects the
# ENTIRE document (ReaderError) if a forbidden C0/DEL control byte appears
# ANYWHERE in the raw stream -- in a plain scalar, inside single- or
# double-quoted scalars, in a comment, or in a field this recognizer never
# even looks at. Checking the raw text (before splitlines(), which itself
# splits on some of these same control bytes -- e.g. \x0b, \x0c, \x1c-\x1e
# -- and would otherwise obscure them) mirrors that exactly: fail closed
# for the whole document rather than risk emitting a token PyYAML would
# have refused.
if _FORBIDDEN_CONTROL.search(raw_text):
return None
lines = raw_text.splitlines()
config = _safe_parse(lines)
if config is _FAIL: