fix(output-encoding): ask about the server half, and say what the family cannot decide

The census recorded this family completing 44 questions on a Node application and finding
nothing, which is what a client-only vocabulary must do there: all six declared sinks were DOM
sinks. NodeGoat's cross-site scripting is server-side.

Decision: the family asks about a value reaching a rendering call, and states in its own
declaration that it cannot decide template-mediated escaping. Whether res.render is safe depends
on the engine's autoescape setting and on the template's markup, neither of which is at the
call, so res.render is not a sink and the family's limits field says so. A new CWE-79 sink
covers res.write, res.end, res.send and the response.* spellings; res.json stays out because an
application/json body is not a document.

Template auto-escaping is claimed where its evidence is, as a configuration defect:
swig.setDefaults, nunjucks.configure and marked.setOptions are security settings now.

It establishes a finding: app/routes/research.js:25, model_entailed,
`req.query.url + req.query.symbol -> res.write(body)`. The handler fetches a URL the caller
chooses and writes the response body into a text/html response, so the document served from
this origin is the attacker's. The record states how coarse the step is: the path from url to
body runs through the client's callback parameter, which the graph treats as reachable from the
call's arguments, so the conclusion rests on the call's semantics rather than on what the graph
established. Five of seven families now establish findings.

Four defects were found on the way, three of them false positives this change would otherwise
have shipped.

- The permissive-value set was shared across every setting, while a boolean's polarity belongs
  to the setting: true leaves the door open in cors({origin: true}) and closes it in
  swig.setDefaults({autoescape: true}). Adding the template engines would have reported the fix
  as the defect on the line that offers it. ConfigSpec now carries per-setting values, and
  Jinja2's Environment moved out of Python's shared list for the same latent reason.
- config.sc matched a dotted setting on its trailing segment alone, so marked.setOptions would
  have claimed an editor's setOptions. dominance.sc already required the qualifier.
- taint.sc asked its flow query of every argument including the receiver at index 0. On a
  response sink the receiver is the response object, so res.end() -- which takes no data -- was
  reported exactly like the res.write(body) two lines above it. Flows now use the real
  arguments, as arity already did.
- The arbiter sorted a file's settings by line number as TEXT, so line 126 preceded line 78 and
  which setting a file reported depended on the spelling of its line numbers.

Template escaping disabled at server.js:135 is detected, measured model_entailed against the
real query and the real arbiter, and still not reported: the arbiter returns one verdict per
question and that file has an earlier permissive setting. Task 14.6 carries it rather than
hiding it.

1,415 tests pass, thirteen skipped; nine Joern-gated query tests pass in the image.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
Marius Ciepluch
2026-09-19 20:38:39 +02:00
co-authored by Claude Opus 5
parent 57f5a3f0dc
commit f31ef862a7
14 changed files with 628 additions and 14 deletions
+14 -2
View File
@@ -38,10 +38,22 @@
val names = split(settingsS)
val function = functionS
def matches(name: String): Boolean = names.exists(n => name == n || name == n.split("\\.").last)
// A dotted spec matches on its trailing segment, which is how the call is named in the graph -- but the
// qualification has to matter, or the segment alone claims every unrelated call that ends the same way.
// `marked.setOptions` would otherwise match an editor's `setOptions` and `nunjucks.configure` any
// `configure` at all. dominance.sc carries this fix and the reason for it; the setting matcher had never
// been given the same treatment, and it became load-bearing when the template engines joined the table.
def matches(name: String, code: String): Boolean =
names.exists { n =>
if (name == n) true
else {
val parts = n.split("\\.")
parts.length > 1 && name == parts.last && parts.init.forall(q => code.contains(q))
}
}
val calls = {
val all = cpg.call.filter(c => matches(c.name)).l
val all = cpg.call.filter(c => matches(c.name, c.code)).l
val inFile = if (fileS.isEmpty) all else all.filter(_.method.filename.endsWith(fileS))
if (function.isEmpty) inFile else inFile.filter(_.method.name == function)
}
+6 -1
View File
@@ -772,7 +772,12 @@
// makes a one-argument interpolated call look like a two-argument bound one -- the exact inversion of the
// test. Real arguments start at index 1.
val realArgs = sink.argument.argumentIndexGt(0).l
val flows = if (hasSources) sink.argument.reachableByFlows(sourceNodes).l else Nil
// Asked of the REAL arguments, for the same reason arity is counted over them. `call.argument` carries the
// receiver at index 0, and on a response sink the receiver is the response object itself: `res.end()` takes
// no data at all, yet `res` is reachable from the request in any handler that captures it, so the flow
// query answered yes about a call that receives nothing. Measured on NodeGoat, where `res.write(body)` is a
// real finding and `res.end()` two lines below it was reported identically.
val flows = if (hasSources) sink.argument.argumentIndexGt(0).reachableByFlows(sourceNodes).l else Nil
// The SHAPE of the sink call, which is what distinguishes a fix from a bug when the fix is a safe form
// rather than a sanitizing call: `execute(sql, params)` binds where `execute(sql + x)` interpolates, and
+28 -3
View File
@@ -40,12 +40,14 @@ def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str =
# Both abstractions read the same literal: a permissive flag ("*", True) and a weak algorithm ("md5").
permissive = {value.strip().strip("'\"").lower() for value in spec.permissive}
permissive |= {value.strip().strip("'\"").lower() for value in spec.weak_algorithms}
scoped = {setting: {value.strip().strip("'\"").lower() for value in values} for setting, values in spec.permissive_by_setting}
for row in settings:
literals = row["literals"]
if not isinstance(literals, list):
continue
allowed = _permissive_for(str(row["setting"]), scoped) or permissive
for literal in literals:
if str(literal).strip().strip("'\"").lower() in permissive:
if str(literal).strip().strip("'\"").lower() in allowed:
return Verdict(
rung=Rung.ENTAILED,
family=spec.family,
@@ -64,6 +66,21 @@ def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str =
return None
def _permissive_for(setting: str, scoped: Mapping[str, set[str]]) -> set[str]:
"""The values permissive for the setting THIS row calls, empty when the facts declare none for it.
A boolean's meaning is a property of the setting, not of the language: `autoescape: false` disables
escaping and `secure: false` disables a cookie flag, while `autoescape: true` is the fix and `origin: true`
is the defect. Asking per setting is what lets the template engines join the table without reporting every
repository that escapes its output.
The longest declared name that appears in the call wins, so a specific `swig.setDefaults` is not decided by
a shorter name that happens to be a substring of the same call.
"""
matched = sorted((name for name in scoped if name in setting), key=len, reverse=True)
return set(scoped[matched[0]]) if matched else set()
def _settings(rows: object, spec: ConfigSpec, *, function: str) -> list[dict[str, object]]:
"""Rows for calls this spec actually names. A non-list (an engine failure) yields nothing."""
if not isinstance(rows, Sequence) or isinstance(rows, (str, bytes)):
@@ -86,8 +103,16 @@ def _settings(rows: object, spec: ConfigSpec, *, function: str) -> list[dict[str
"literals": [str(v) for v in literals] if isinstance(literals, Sequence) and not isinstance(literals, str) else [],
}
)
# Deterministic order: the first row decides, so it must not depend on engine ordering.
return sorted(kept, key=lambda row: (str(row["method"]), str(row["line"]), str(row["setting"])))
# Deterministic order: the first row decides, so it must not depend on engine ordering. The line sorts
# NUMERICALLY, which is what "the first row" was always meant to mean: as text, line 126 precedes line 78,
# and which of a file's settings gets reported was decided by the spelling of its line number. Caught when
# a second permissive setting was added to a file that already had one.
def _line(row: dict[str, object]) -> tuple[int, str]:
text = str(row["line"])
return (int(text) if text.lstrip("-").isdigit() else 1 << 30, text)
return sorted(kept, key=lambda row: (str(row["method"]), _line(row), str(row["setting"])))
__all__ = ["request_params", "verdict"]
+14
View File
@@ -173,6 +173,13 @@ class ConfigSpec:
# algorithm NAMED, not a setting left open. The data has been on the sink facts (`weak_literals`) since
# before this feature and nothing read it.
weak_algorithms: tuple[str, ...] = ()
# Which values are permissive FOR WHICH setting, as (setting, values) pairs. A flat set cannot hold the
# answer, because the polarity of a boolean is per setting: `true` leaves the door open in
# `cors({origin: true})` and closes it in `swig.setDefaults({autoescape: true})`. With one shared set,
# adding a template engine would have reported the fix as the defect on exactly the line whose comment
# offers it. A setting a discharger names uses that discharger's values; a setting no discharger names
# falls back to `permissive`, which is the committed behaviour for `app.listen` and `createClient`.
permissive_by_setting: tuple[tuple[str, tuple[str, ...]], ...] = ()
def config_specs(
@@ -194,6 +201,12 @@ def config_specs(
weak_calls = tuple(sorted({call for sink in flow.sinks if sink.weak_literals for call in sink.calls}))
if not settings or not permissive:
return {}
scoped_values: dict[str, set[str]] = {}
for fact in scoped.dischargers:
if fact.kind != "non_permissive_value" or not fact.permissive_values:
continue
for call in fact.calls:
scoped_values.setdefault(call, set()).update(fact.permissive_values)
return {
"config_secrets": ConfigSpec(
family="config_secrets",
@@ -201,6 +214,7 @@ def config_specs(
settings=tuple(sorted(set(settings) | set(weak_calls))),
permissive=permissive,
weak_algorithms=weak,
permissive_by_setting=tuple((call, tuple(sorted(values))) for call, values in sorted(scoped_values.items())),
)
}
+1
View File
@@ -46,6 +46,7 @@ description = "Untrusted data reaches a rendered document or a header without en
cwes = ["CWE-79", "CWE-80", "CWE-83", "CWE-113", "CWE-116"]
mechanisms = []
verifier = "none"
limits = """Models a value reaching a rendering CALL -- a DOM sink, or a response body written directly. It cannot decide template-mediated escaping, which is the commonest shape a server-rendered application has: whether `res.render` is safe depends on the template engine's autoescape configuration and on the template's own markup, neither of which is at the call. That setting is claimed instead as a configuration defect, where a permissive literal can be read: `swig.setDefaults`, `nunjucks.configure` and Jinja2's `Environment` are security settings in the obligation facts, and turning escaping off is reported by config_secrets. Response splitting (CWE-113) is claimed by no sink here at all, because modern Node rejects a newline in a header value and the claim would be about an engine this project does not model. A scan of a server-rendered application that reports nothing from this family has established that no tainted value reached a modelled rendering call, and nothing about whether its templates escape."""
[[family]]
id = "untrusted_destination"
@@ -38,6 +38,20 @@ resource_arg = 0
requires = ["non_permissive_value"]
sensitivity = "medium"
[[operation]]
# Template auto-escaping, which is the setting NodeGoat's cross-site scripting actually turns off:
# `swig.setDefaults({ autoescape: false })` at server.js:135, with `autoescape: true` sitting commented out
# directly beneath it. The 2026-09-19 family census recorded output encoding completing 44 questions on that
# repository and finding nothing, because escaping there is a property of the engine's configuration rather
# than of any call a value flows into. It is a value-shaped defect, so it is claimed here rather than by a
# flow, and `output_encoding` states this limit in its own declaration.
id = "template-escaping"
kind = "security_setting"
calls = ["swig.setDefaults", "nunjucks.configure", "marked.setOptions"]
resource_arg = 0
requires = ["non_permissive_value"]
sensitivity = "medium"
[[discharger]]
id = "route-guard"
kind = "path_guard"
@@ -62,6 +76,16 @@ kind = "non_permissive_value"
calls = ["cors", "cookie", "session", "helmet", "setHeader"]
permissive_values = ["'*'", "\"*\"", "true", "0.0.0.0", "null", "false", "origin: true", "secure: false", "httpOnly: false"]
[[discharger]]
# The polarity of a boolean belongs to the setting, not to the language. `true` leaves the door open in
# `cors({origin: true})` and closes it in `swig.setDefaults({autoescape: true})`, so these settings carry
# their OWN permissive value rather than sharing the general one -- which holds both `true` and `false`, and
# would have reported NodeGoat's commented-out fix as the defect.
id = "safe-template-escaping"
kind = "non_permissive_value"
calls = ["swig.setDefaults", "nunjucks.configure", "marked.setOptions"]
permissive_values = ["false"]
[[discharger]]
id = "validated-input"
kind = "validated_input"
@@ -68,9 +68,18 @@ id = "safe-setting"
kind = "non_permissive_value"
calls = ["CORS", "cross_origin", "set_cookie", "app.run", "config.update", "add_middleware",
"session_setup", "SimpleCookieStorage", "setup_session", "set_secure_cookie",
"Application", "RedisStorage", "Environment"]
"Application", "RedisStorage"]
permissive_values = ["'*'", "\"*\"", "True", "0.0.0.0", "None", "False", "allow_all", "disable"]
[[discharger]]
# Jinja2's `Environment(autoescape=...)`, moved out of the general list because `True` is in it and `True` is
# the SAFE value here. Sharing one permissive set made the correct configuration indistinguishable from the
# dangerous one, which is the same defect the JavaScript template engines would have introduced.
id = "safe-template-escaping"
kind = "non_permissive_value"
calls = ["Environment", "select_autoescape"]
permissive_values = ["False"]
[[discharger]]
id = "validated-input"
kind = "validated_input"
@@ -38,6 +38,20 @@ resource_arg = 0
requires = ["non_permissive_value"]
sensitivity = "medium"
[[operation]]
# Template auto-escaping, which is the setting NodeGoat's cross-site scripting actually turns off:
# `swig.setDefaults({ autoescape: false })` at server.js:135, with `autoescape: true` sitting commented out
# directly beneath it. The 2026-09-19 family census recorded output encoding completing 44 questions on that
# repository and finding nothing, because escaping there is a property of the engine's configuration rather
# than of any call a value flows into. It is a value-shaped defect, so it is claimed here rather than by a
# flow, and `output_encoding` states this limit in its own declaration.
id = "template-escaping"
kind = "security_setting"
calls = ["swig.setDefaults", "nunjucks.configure", "marked.setOptions"]
resource_arg = 0
requires = ["non_permissive_value"]
sensitivity = "medium"
[[discharger]]
id = "route-guard"
kind = "path_guard"
@@ -62,6 +76,16 @@ kind = "non_permissive_value"
calls = ["cors", "cookie", "session", "helmet", "setHeader"]
permissive_values = ["'*'", "\"*\"", "true", "0.0.0.0", "null", "false", "origin: true", "secure: false", "httpOnly: false"]
[[discharger]]
# The polarity of a boolean belongs to the setting, not to the language. `true` leaves the door open in
# `cors({origin: true})` and closes it in `swig.setDefaults({autoescape: true})`, so these settings carry
# their OWN permissive value rather than sharing the general one -- which holds both `true` and `false`, and
# would have reported NodeGoat's commented-out fix as the defect.
id = "safe-template-escaping"
kind = "non_permissive_value"
calls = ["swig.setDefaults", "nunjucks.configure", "marked.setOptions"]
permissive_values = ["false"]
[[discharger]]
id = "validated-input"
kind = "validated_input"
@@ -57,6 +57,22 @@ cwe = "CWE-79"
calls = ["innerHTML", "outerHTML", "insertAdjacentHTML", "document.write", "dangerouslySetInnerHTML", "v-html"]
rule_ids = []
[[sink]]
# CWE-79, the SERVER half. Every sink above is a DOM sink, so this family could only ever ask about code
# running in a browser -- and the 2026-09-19 family census recorded it completing 44 questions on a Node
# application and finding nothing, which is exactly what a client-only vocabulary must do there. These are the
# documented ways an Express or raw-Node handler puts bytes in a response body without an encoder in between.
#
# `res.json` is deliberately absent: it sets an application/json content type, so a value reaching it is not
# a script in a document. `res.render` is deliberately absent too, and for a different reason -- whether a
# template escapes is a property of the engine's configuration rather than of the call, so it is claimed as a
# configuration defect (`swig.setDefaults`, `nunjucks.configure` in the obligation facts) and stated as a
# limit on this family rather than guessed at here.
id = "html.response"
cwe = "CWE-79"
calls = ["res.write", "res.end", "res.send", "response.write", "response.end", "response.send"]
rule_ids = []
[[sink]]
# CWE-22. Node filesystem entry points that accept a caller-supplied path.
id = "path.open"