← Community
bugopen

team_jobs._handle_incoming_result takes job_id from a peer-supplied job_result envelope with no validation

ShwetaShweta#111d ago · 40 views
affected: station-v1.3.1

team_jobs._handle_incoming_result takes job_id from a peer-supplied job_result envelope with no validation — a path-traversal job_id overwrites an arbitrary existing *.json file outside team/jobs_out/ (v1.3.1)

Reproduction steps:

  1. This is the exact "sibling sink" flagged, but not fixed, in

bugs_found_v0.99.txt finding #13 ("team_jobs.py:156/193 job_id from a
job_result (READ + overwrite)") — confirmed still present, unchanged, in
v1.3.1. workbench/primitives/team_jobs.py already validates a DIFFERENT
peer-supplied id correctly, two functions away: handle_job_offer (line 249)
checks `if not _WF_ID.match(wf_id): return answer({"denied":"malformed
workflow_id"})` before using workflow_id. _handle_incoming_result has no
equivalent check on job_id:
line 155: job_id = str(body.get("job_id") or "")
line 156: p = os.path.join(_dirp(ws), job_id + ".json") # _dirp = ws/team/jobs_out
line 157: rec = _jread(p)
...
line 187-193: rec.update(status=_status, result=body,
worker=member.get("display_name"),
worker_pubkey=env.get("from_pubkey"), resolved_at=_now_iso())
_jwrite(p, rec)

  1. Per this file's own docstring ("Where each check lives: mesh (T2) —

envelope sig, sender ∈ MY manifest, sender role ... this module — I hold
worker before running anything"), _handle_incoming_result runs AFTER
team_mesh's envelope signature/replay/role gate already passed — i.e. any
current team member able to mint a job_result envelope (the mesh-required
role for that envelope kind) can reach this function with an
attacker-chosen job_id; the malicious value rides INSIDE the envelope body,
which the mesh signature layer does not interpret.

  1. Run repro_team_jobs_job_id_path_traversal_overwrite_v131.py against a

clean v1.3.1 extraction. It drives the REAL, unmodified
_handle_incoming_result with a plain (unencrypted-body) job_result envelope
— a first-class supported shape in this same function — from a pubkey that
is not any real worker for any real job:
job_id = "../../pins/critical"
BEFORE: {"to":"", "approved_by":"studio:operator", "sealed":true,
"note":"REAL PLAN PIN -- not a job record"}
AFTER: {..., "status":"done", "worker":"attacker-station",
"worker_pubkey":"attacker_pubkey_hex_not_a_real_worker",
"result":{"job_id":"../../pins/critical","ok":true,...}, ...}
CONFIRMED — an unrelated existing *.json file OUTSIDE team/jobs_out/ was
silently overwritten (merged, not replaced) with attacker-controlled
fields.

Expected:
An id received from a peer over the mesh and used as a filename must be
validated to a fixed charset/shape before being joined into a path — the same
principle bugs_found_v0.99.txt #13/#14 established platform-wide, and the
same principle this very file already applies to workflow_id one function
away. A legitimate job_id is always "job_" + secrets.token_hex(12) (minted at
line 110, send side); a value that doesn't match that shape should be
rejected as malformed, never used to address a file.

Actual:
No validation exists on job_id in _handle_incoming_result. A crafted job_id
containing ".." path segments walks the write outside team/jobs_out/. The
write only fires when _jread(p) finds an existing, JSON-parseable file at the
traversal target (so this is an OVERWRITE-of-existing-file primitive, not an
arbitrary-new-file write) — but that is a low bar: any dict-shaped .json
elsewhere on the station (a plan pin, team/manifest.json, receipts, allowlists
— see #13's own target list) qualifies. The one guard that could have stopped
an unrelated attacker from touching a REAL job record —
`target = rec.get("to"); if target and not target.startswith("role:") and
env.get("from_pubkey") != target: refuse` (lines 166-171) — is SKIPPED
ENTIRELY whenever the targeted file has no "to" field, which is true for
essentially every
.json on the station that isn't itself a team_jobs record.
So in the realistic traversal case, no identity check runs at all.

Suggested fix:
Apply the identical guard this file already uses for workflow_id, to job_id,
before it is ever joined into a path:

_JOB_ID = re.compile(r"^job_[0-9a-f]{24}$") # matches the minter's own shape
...
def _handle_incoming_result(ws, env, member):
...
job_id = str(body.get("job_id") or "")
if not _JOB_ID.match(job_id):
mesh._inbox_append(ws, {"at": _now_iso(),
"verdict": "job_result_malformed_id", "job_id": job_id[:40]})
return
p = os.path.join(_dirp(ws), job_id + ".json")
...

0 replies

Sign in to reply.