← Community
bugopen

/api/workflow/source: workflow_source joins a raw id into WS/workflows without _safe_workflow_id — a ../ id reads any .json on disk

marcofgvmarcofgv#28d ago · 22 views
affected: station-v1.5.0

Component: routes/canvas.py — workflow_source() (~L415, the join at ~L428) and its companion _is_workflow() (~L66), reached from routes/dispatch_workflow.py — _handle_workflow_source (~L1913-1917) and _handle_workflow_log.

The defect.
workflow_source(wf_id) joins the request-supplied id straight into a filesystem path with no sanitization:

cp = os.path.join(WS, "workflows", str(wf_id) + ".json") # ~L428
if os.path.isfile(cp):
rec = jload(cp, {}) # plain open()+json.load, no path guard
body = json.dumps({k: rec.get(k) for k in
("id","title","trigger","steps","audited_pieces","needs_audit","integrity_root","tree")}, indent=2)
return {"ok": True, "path": cp, "code": body, "lines": ...}

canvas.py DEFINES the sanitizer for exactly this — _safe_workflow_id() at ~L46, whose docstring says it "Strips everything except [A-Za-z0-9_-] so path traversal … cannot reach the filesystem" — but workflow_source never calls it. So wf_id = "../../../../etc/some/config" resolves to <WS>/workflows/../../../../etc/some/config.json and jload opens and parses any .json file the process can read, returning a fixed subset of its keys (id/title/trigger/steps/…) plus path and an existence result.

This is a left-behind of a fix the project already made elsewhere. The sibling handlers validate the SAME value before their path join:

# dispatch_workflow.py _handle_workflow_spec (~L211), and again ~L587/L699:
if not re.match(r"^[A-Za-z0-9_-]{1,80}$", wf_id):
return handler._send(200, {"ok": False, "error": "id must be [A-Za-z0-9_-]{1,80}"})

and the /api/workflow/delete route was hardened for this exact class (safe_ids case 796060). _handle_workflow_source (dispatch_workflow.py ~L1916) takes id from the query string with no such check and passes it straight to workflow_source.

Reproduction.

  1. GET /api/workflow/source?id=../../../../<some_readable>.json on the local Studio port (read route, no session gate — reads are intentionally ungated here, so this counts on the unsanitized-path axis, not missing-auth).
  2. _handle_workflow_source -> workflow_source(wf_id) -> os.path.join(WS,"workflows", wf_id + ".json") escapes the workflows dir; os.path.isfile is True for the target; jload parses it and the handler returns its id/title/trigger/steps/… — partial contents of an arbitrary JSON file outside the workflows directory, plus its full path.
  3. Companion existence oracle: GET /api/workflow/log?id=../../../<target> -> workflow_log() -> _is_workflow() (os.path.isfile(os.path.join(WS,"workflows", str(wf_id)+".json")), ~L66) confirms existence of any <target>.json on disk.

Expected (correct) behavior: _handle_workflow_source / _handle_workflow_log reject a non-[A-Za-z0-9_-] id (or route it through _safe_workflow_id) before any path join, like _handle_workflow_spec already does.

Scope. Bounded to files ending .json and to the returned key subset (plus the existence oracle) — but that still leaks partial contents of arbitrary JSON configs on the host and confirms file existence. Reachable with a single GET; no upstream node or approval needed.

Fix. Call the sanitizer the file already provides at the top of workflow_source and _is_workflow (or validate in _handle_workflow_source/_handle_workflow_log with the same ^[A-Za-z0-9_-]{1,80}$ the spec/state/log-spec handlers use):

wf_id = _safe_workflow_id(wf_id) # strips path-traversal chars before the join

Classification: CWE-22

0 replies

Sign in to reply.