T09 · Insecure Skill Coding Practices
Note
- Location
- scripts/bnb_band_bot.py:46
- Finding
- Unnecessary Private-Key Loading in Placeholder and Dry-Run Modes<![CDATA[ ## Vulnerability Details **File Location**: `scripts/bnb_band_bot.py`, lines 46–50 **Vulnerability Type**: Unnecessary handling of sensitive credentials **Risk Level**: Low ### Vulnerable Code ```python def run(dry_run: bool): pk = need("EVM_PRIVATE_KEY") validate_pk(pk) need("BNB_RPC_URL") token_in = os.getenv("TOKEN_IN", "WBNB") token_out = need("TOKEN_OUT") ``` The private key is not subsequently used. The swap implementation confirms that neither dry-run nor nominal live mode signs or submits a transaction: ```python def mock_swap(side: str, amount_bnb: Decimal, dry_run: bool): if dry_run: log(f"[DRY-RUN] swap {side} {amount_bnb} BNB") return # TODO: implement real tx signing + router call with web3.py log(f"[LIVE-PLACEHOLDER] swap {side} {amount_bnb} BNB") ``` ### Technical Analysis Both `--mode dry-run` and `--mode run` require `EVM_PRIVATE_KEY`, retrieve it from the process environment, and retain it in the local `pk` variable for the lifetime of the infinite trading loop. This violates data-minimization and least-exposure principles because the current implementation never uses the key for signing. No code in the audited project logs, transmits, writes, or otherwise exfiltrates the private key. Therefore, this is not evidence of embedded malicious behavior. Nevertheless, requiring a production wallet credential for functionality that only logs placeholder swaps unnecessarily expands the credential's exposure to the Python process, debuggers, process inspection available to sufficiently privileged users, instrumentation, and potential future memory-disclosure defects. ### Attack Path 1. An operator follows `SKILL.md` and exports a funded wallet's private key as `EVM_PRIVATE_KEY`. 2. The operator starts either dry-run or placeholder live mode. 3. The script copies the credential from the environment into a Python string and keeps the process running indefinitely. 4. An attacker who has alre ...[truncated 1026 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not request or load `EVM_PRIVATE_KEY` in dry-run mode. 2. Until transaction signing is implemented, remove the private-key requirement from live placeholder mode as well, or reject `--mode run` with an explicit message that live execution is unsupported. 3. When real signing is introduced, load the credential only immediately before signing and avoid retaining it throughout the polling loop. 4. Prefer a dedicated low-value trading wallet, external signer, hardware wallet, or managed signing service instead of a long-lived plaintext environment variable. 5. Never log the key or include it in exception messages, diagnostics, subprocess arguments, or telemetry. 6. Document that placeholder live mode does not execute transactions so operators are not encouraged to provide production credentials unnecessarily. A safer mode-specific pattern is: ```python def run(dry_run: bool): need("BNB_RPC_URL") if not dry_run: raise RuntimeError( "Live execution is unavailable until secure transaction signing is implemented" ) token_in = os.getenv("TOKEN_IN", "WBNB") token_out = need("TOKEN_OUT") ``` ]]>