T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/xr-call:37
- Finding
- Insufficient Validation and Failure-Stop Enforcement for Physical Movement Commands<![CDATA[ ## Vulnerability Details **File Location**: `scripts/xr-call:37-61`, `scripts/xr-call:128-164` **Vulnerability Type**: Missing input validation and incomplete fail-safe handling for actuator commands **Risk Level**: High ### Vulnerable Code Argument parsing at `scripts/xr-call:37-61`: ```python def _parse_args() -> dict[str, Any]: """解析 argv[2:] 中的 --key value 参数。""" kwargs: dict[str, Any] = {} i = 2 while i < len(sys.argv): arg = sys.argv[i] if not arg.startswith("--"): print(json.dumps({"ok": False, "message": f"参数必须以 -- 开头: {arg}"}), file=sys.stderr) sys.exit(1) key = arg[2:].replace("-", "_") if i + 1 < len(sys.argv) and not sys.argv[i + 1].startswith("--"): raw = sys.argv[i + 1] i += 2 # 尝试转数值 try: raw = int(raw) except ValueError: try: raw = float(raw) except ValueError: pass kwargs[key] = raw else: kwargs[key] = True i += 1 return kwargs ``` Movement and turning dispatch at `scripts/xr-call:128-164`: ```python elif cmd == "move": import rclpy from xrrobot_offline_voice.mcp_handlers import move as _move ctx = _init_ros_and_context("xrrobot_offline_voice") kwargs = _parse_args() direction = kwargs.pop("direction", "") if direction not in ("forward", "backward"): print(json.dumps({"ok": False, "message": "direction 必须是 forward 或 backward"})) _shutdown(ctx.ros_node, rclpy) sys.exit(1) try: result = _move(direction=direction, context=ctx, **kwargs) print(json.dumps(result, ensure_ascii=False, default=str)) except Exception as e: print(json.dumps({"ok": False, "message": str(e)})) _shutdown(ctx.ros_node, rclpy) eli ...[truncated 3811 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Define an explicit argument schema for every subcommand. Reject unknown keys rather than forwarding arbitrary `**kwargs`. 2. Require movement parameters to be real, finite numbers: ```python import math def require_finite_number(value, name): if isinstance(value, bool) or not isinstance(value, (int, float)): raise ValueError(f"{name} must be numeric") value = float(value) if not math.isfinite(value): raise ValueError(f"{name} must be finite") return value ``` 3. Enforce conservative ranges appropriate for the hardware, such as administrator-defined maximum duration, linear speed, and angular speed. Reject zero or negative durations. 4. Do not rely exclusively on the imported handler for safety validation. Validate parameters again at the local actuator boundary. 5. Guarantee stop-on-failure and stop-on-interruption. For `move` and `turn`, invoke the stop handler in exception recovery and handle `KeyboardInterrupt`, termination signals, and shutdown failures. If the normal movement handler already stops on success, preserve that behavior while ensuring that exceptional paths also stop. 6. Use a motor-controller or ROS watchdog that automatically commands zero velocity when fresh bounded commands stop arriving. Application-level exception handling should not be the only safety mechanism. 7. Return a nonzero exit status when handler execution fails so orchestration layers cannot mistake a JSON error printed with exit status zero for successful execution. 8. Add tests covering excessive values, negative values, `nan`, positive and negative infinity, valueless flags, unknown arguments, handler exceptions after movement starts, process interruption, and verification that every failure path sends a stop command. ]]>