T09 · Insecure Skill Coding Practices
Note
- Location
- scripts/main.py:10
- Finding
- Negative Transport Duration Produces a Misleading Low-Risk Assessment<![CDATA[ ## Vulnerability Details **File Location**: `scripts/main.py`, lines 10–31 **Vulnerability Type**: Insufficient numeric input validation **Risk Level**: Low ### Vulnerable Code ```python def calculate_risk(route, duration_hours, packaging): """Calculate cold chain risk.""" # Simplified risk calculation base_risk = duration_hours * 0.5 packaging_factor = {"dry-ice": 0.8, "liquid-nitrogen": 0.3, "gel-packs": 1.2} risk = base_risk * packaging_factor.get(packaging, 1.0) print(f"Route: {route}") print(f"Duration: {duration_hours} hours") print(f"Packaging: {packaging}") print(f"Risk score: {risk:.2f}") if risk < 10: return "Low risk" elif risk < 20: return "Medium risk" else: return "High risk" ``` ```python parser.add_argument("--duration", "-d", type=int, required=True, help="Duration in hours") ``` ### Technical Analysis The `--duration` argument is converted to an integer, but no minimum-value constraint is enforced. Consequently, zero and negative durations are accepted and used directly in the risk calculation. Because the calculated base risk is `duration_hours * 0.5`, a negative duration produces a negative risk score. Any negative score satisfies the `risk < 10` condition and is therefore classified as `"Low risk"`. This is an input-validation and business-logic integrity flaw. It does not permit arbitrary code execution or privilege escalation, but it can cause the tool to produce an invalid and potentially unsafe operational assessment. ### Attack Path 1. Invoke the calculator with a negative duration: ```bash python scripts/main.py --route "NYC-Boston" --duration -100 --packaging dry-ice ``` 2. The argument parser accepts `-100` because it is a syntactically valid integer. 3. The calculator computes: - Base risk: `-100 * 0.5 = -50` - Packaging-adjusted risk: `-50 * 0.8 = -40` 4. Since `-40` is less than `10`, the application reports `"Low ...[truncated 682 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Implement an argparse type validator that rejects non-positive durations before risk calculation: ```python def positive_int(value): try: parsed = int(value) except ValueError as exc: raise argparse.ArgumentTypeError("Duration must be an integer") from exc if parsed <= 0: raise argparse.ArgumentTypeError("Duration must be greater than zero") return parsed parser.add_argument( "--duration", "-d", type=positive_int, required=True, help="Duration in hours; must be greater than zero", ) ``` Apply defense in depth by also validating `duration_hours` inside `calculate_risk`, so direct callers cannot bypass command-line validation: ```python if duration_hours <= 0: raise ValueError("Duration must be greater than zero") ``` Add automated tests covering negative values, zero, the minimum accepted duration, unusually large values, and non-integer input. If the domain has a realistic maximum transport duration, enforce and document that upper bound as well. ]]>