IBM / unitxt / 41817116053

78%

push

github

Security: Pass the validated name onward in metrics service (#cs-12, #cs-13, #cs-14, #cs-18) (#1995)

Remediates the 4 py/path-injection CodeQL alerts (HIGH, CWE-22) still open after
the previous fix, reported in the IBM OSPO security notification (#1994):
cs-12, cs-13, cs-14 (src/unitxt/artifact.py:238,240) and cs-18
(src/unitxt/utils.py:222).

The earlier fix (#1993) blocked the attack at runtime but left the alerts open,
and the alerts were right to stay open. It validated a derived value and then
passed the original one on:

    requested_name, _ = separate_inside_and_outside_square_brackets(metric_name)
    if not is_name_legal_for_catalog(requested_name):
        raise HTTPException(status_code=400, ...)
    ...
    metric = ArtifactFetcherMixin.get_artifact(metric_name)   # <- raw value

Checking requested_name says nothing about metric_name, so nothing about the
value that actually reached the loader. It happened to be safe only because the
two strings agreed on the part that mattered -- a property no check enforced and
any later edit could break. CodeQL reported it as unsanitized because, as
dataflow, it is. The taint path in the current SARIF is unchanged from before
the fix:

  service/metrics/main.py:69,100  metric_name (FastAPI path parameter)
  operators.py:1190-1192          fetch_artifact(artifact_identifier)
  artifact.py:546,562-563         fetch_artifact(artifact_rep)
  artifact.py:238,240             open(path)            <- cs-12, cs-13, cs-14
  utils.py:203-204,221-222        open(path)            <- cs-18

cs-18 is a fresh alert raised against the fix commit itself, replacing cs-15 and
cs-16 at the same sink: the flow survived, only its line numbers moved.

The fix makes the sanitization real rather than incidental:

1. artifact.py -- new sanitize_catalog_identifier().

   Validates a reference as a catalog name and returns the name it validated,
   rebuilt from its checked segments, plus the bracketed overrides already
   parse... (continued)

2793 of 4026 branches covered (69.37%)

Branch coverage included in aggregate %.

10996 of 13569 relevant lines covered (81.04%)

0.81 hits per line