T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/genrandom.py:4
- Finding
- Predictable PRNG Used for Security-Sensitive Passwords and Random Values<![CDATA[ ## Vulnerability Details **File Location**: `scripts/genrandom.py`, lines 4-29 **Vulnerability Type**: Use of a non-cryptographic pseudorandom number generator for security-sensitive values **Risk Level**: High ### Vulnerable Code ```python import random import string import sys import uuid def random_int(min_val: int, max_val: int) -> int: """Generate random integer.""" return random.randint(min_val, max_val) def random_float(min_val: float, max_val: float) -> float: """Generate random float.""" return random.uniform(min_val, max_val) def random_string(length: int, charset: str = None) -> str: """Generate random string.""" if charset is None: charset = string.ascii_letters + string.digits return ''.join(random.choice(charset) for _ in range(length)) def random_password(length: int) -> str: """Generate secure password.""" chars = string.ascii_letters + string.digits + "!@#$%^&*()_+-=" return ''.join(random.choice(chars) for _ in range(length)) ``` ### Technical Analysis The implementation uses Python's `random` module for integers, strings, and passwords. This module is based on the deterministic Mersenne Twister pseudorandom number generator and is explicitly unsuitable for cryptographic purposes. This is security-relevant because: - `random_password()` describes its output as a secure password. - `SKILL.md` advertises the Skill as generating “cryptographically secure random values.” - The documentation identifies security tokens as an intended use case. If an attacker obtains enough PRNG-derived output or otherwise learns the generator state, future outputs may become predictable. Random strings or passwords derived from the same process must therefore not be treated as cryptographic secrets. Operating-system seeding does not make Mersenne Twister a cryptographically secure generator because its internal state and output transformation are not designed to resist prediction. The UUID pa ...[truncated 1607 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace security-sensitive use of `random.choice()` with `secrets.choice()`: ```python import secrets import string def random_string(length: int, charset: str | None = None) -> str: if length < 0: raise ValueError("Length must be non-negative") if charset is None: charset = string.ascii_letters + string.digits if not charset: raise ValueError("Character set must not be empty") return ''.join(secrets.choice(charset) for _ in range(length)) def random_password(length: int) -> str: if length <= 0: raise ValueError("Password length must be positive") chars = string.ascii_letters + string.digits + "!@#$%^&*()_+-=" return ''.join(secrets.choice(chars) for _ in range(length)) ``` 2. If integer output may be used for security purposes, generate it with rejection sampling through `secrets.randbelow()`: ```python def secure_random_int(min_val: int, max_val: int) -> int: if min_val > max_val: raise ValueError("Minimum must not exceed maximum") return min_val + secrets.randbelow(max_val - min_val + 1) ``` 3. Do not use floating-point random values as cryptographic tokens. Generate a fixed number of random bytes with `secrets.token_bytes()`, `secrets.token_hex()`, or `secrets.token_urlsafe()` instead. 4. Clearly separate test-data generation from cryptographic generation. If non-security randomness remains available, label it explicitly as unsuitable for passwords, tokens, keys, session identifiers, or other secrets. 5. Add automated tests that verify security-sensitive functions use the `secrets` or operating-system randomness APIs rather than the global `random` module. 6. Correct the CLI documentation so that documented options match the implementation, and retain cryptographic-security claims only after all relevant generation paths use cryptographically secure APIs. ]]>