T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/edge_cpu_tuner.py:159
- Finding
- Subprocess Output Limit Is Enforced Only After Unbounded Memory Buffering<![CDATA[ ## Vulnerability Details **File Location**: `scripts/edge_cpu_tuner.py`, lines 159–167 **Vulnerability Type**: Improper resource control leading to local denial of service **Risk Level**: Medium ### Vulnerable Code ```python proc = subprocess.run( list(argv), shell=False, cwd=cwd, env=safe_env(), text=True, encoding="utf-8", errors="replace", stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=timeout_value, check=False, ) output_size = len(proc.stdout.encode("utf-8", "replace")) + len(proc.stderr.encode("utf-8", "replace")) if output_size > max_output_bytes: raise TunerError(f"process output exceeded {max_output_bytes} bytes: {argv[0]}", 1) ``` ### Technical Analysis The process is launched with both `stdout` and `stderr` connected to `subprocess.PIPE`. Python's `subprocess.run()` internally waits for the process and collects its complete output before returning. The configured `max_output_bytes` limit is checked only after `subprocess.run()` has completed and all output has already been buffered in memory. Therefore, the limit constrains only whether the output is accepted after execution; it does not constrain peak memory consumption. A faulty or hostile benchmark executable can continuously emit data until the timeout expires. During that period, Python may retain substantially more than the documented default limit of 8 MiB. Sufficient output can exhaust available memory, invoke the operating system's out-of-memory handling, or destabilize other workloads on a constrained edge host. The timeout limits execution duration but does not provide a reliable memory bound because output can be generated rapidly. Additionally, the implementation does not establish a dedicated process group, so subprocess descendants are not explicitly terminated as a group when a timeout or interruption occurs. ### Attack Path 1. An attacker persuades a user or automation system to run `bench` with a faulty or att ...[truncated 1507 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Replace the fully buffered `subprocess.run()` call with bounded, incremental output collection: 1. Launch the benchmark using `subprocess.Popen`. 2. Read stdout and stderr incrementally without allowing either stream to block the other. Suitable approaches include: - `selectors` with binary, nonblocking pipes on supported platforms; - dedicated reader threads with a shared byte budget; or - temporary files combined with active size monitoring. 3. Track the combined raw byte count before decoding. 4. As soon as the combined output exceeds `max_output_bytes`: - stop reading additional output; - terminate the subprocess; - wait for a short grace period; - forcefully kill it if it does not exit; and - return a controlled output-limit error. 5. Start the subprocess in a dedicated process group or session where supported. On timeout, interruption, or output overflow, terminate the complete group so descendants cannot remain active. 6. Retain only a bounded stderr tail for diagnostics rather than buffering the full stream. 7. Add regression tests using a fake benchmark that: - emits more than the configured byte limit; - emits simultaneously to stdout and stderr; - continues emitting until terminated; and - spawns a child process, to verify process-tree cleanup. 8. Document platform-specific process-group behavior if cross-platform support is required. The output limit must be enforced while bytes are being received, not after the child process has completed. ]]>