T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/render_cuts.py:20
- Finding
- Unvalidated EDL Values Allow FFmpeg Filter-Graph Injection in the Hard-Cut Renderer<![CDATA[ ## Vulnerability Details **File Location**: `scripts/render_cuts.py`, lines 20-26 and 42-60 **Vulnerability Type**: FFmpeg filter-graph injection through untrusted EDL data **Risk Level**: Medium ### Vulnerable Code ```python plan = json.loads(args.edl.read_text()) inputs = plan["inputs"] deltas = plan.get("deltas", [0.0] * len(inputs)) edl = plan["edl"] audio_src = plan["audio_source"] W, H = args.width, args.height ``` ```python filters = [] for i, row in enumerate(edl): cam = row["cam"]; start = row["start"]; end = row["end"] filters.append( f"[{cam}:v]trim=start={start}:end={end},setpts=PTS-STARTPTS," f"scale={W}:{H}:force_original_aspect_ratio=decrease," f"pad={W}:{H}:(ow-iw)/2:(oh-ih)/2,setsar=1,fps=30[v{i}]" ) concat_inputs = "".join(f"[v{i}]" for i in range(len(edl))) filters.append(f"{concat_inputs}concat=n={len(edl)}:v=1:a=0[vout]") fc = ";".join(filters) # Audio: trim from the EDL's first-row start (so window EDLs work) audio_offset = edl[0]["start"] if edl else 0.0 duration = plan["duration_sec"] fc += (f";[{audio_src}:a:0]atrim=start={audio_offset}:" f"duration={duration},asetpts=PTS-STARTPTS[aout]") ``` ### Technical Analysis The renderer parses an EDL JSON document and directly interpolates the following fields into an FFmpeg `-filter_complex` expression: - `edl[].cam` - `edl[].start` - `edl[].end` - `audio_source` - `duration_sec` The code does not verify that camera identifiers are bounded integers or that time values are finite numbers. JSON therefore permits an attacker-controlled string to reach the FFmpeg filter parser. Characters such as brackets, commas, semicolons, colons, and option separators have structural meaning inside an FFmpeg filter graph. A crafted value can terminate the intended expression and append or modify filter chains. The use of `subprocess.run(cmd, check=True)` with an argument list prevents operating-system shell injection, but it does not prevent injection ...[truncated 1966 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate the complete EDL against a strict schema before constructing any FFmpeg command: - Require `inputs` to be a non-empty list of strings. - Require `audio_source` and every `cam` value to be integers, explicitly rejecting booleans. - Enforce `0 <= audio_source < len(inputs)` and `0 <= cam < len(inputs)`. - Require `start`, `end`, `duration_sec`, and every delta to be finite numeric values. - Enforce `0 <= start < end <= duration_sec`. - Reject empty EDLs, unknown fields where practical, and mismatched `inputs`, `deltas`, and `coverage` lengths. 2. Convert validated numbers to canonical representations before interpolation. Do not accept arbitrary strings for numeric graph parameters. 3. Set reasonable limits on: - Number of EDL segments. - Total duration. - Individual segment duration. - Output resolution, bitrate, and frame rate. 4. Reject non-finite floating-point values with `math.isfinite()`. 5. Consider generating a filter-complex script from validated primitives. A script file does not replace validation, but it makes graph construction and review clearer. 6. Run FFmpeg with least privilege in a restricted environment when processing EDLs from untrusted sources. Where feasible, limit filesystem access, networking, runtime, memory, CPU, and output size. ]]>