T09 · Insecure Skill Coding Practices
- Location
scripts/update_account.py:16- Finding
Incomplete Price Data Is Persisted as Authoritative Account Valuation
- Content
View full analysis
Vulnerability Details
File Location:
scripts/update_account.py, lines 16–23 and 32–40
Vulnerability Type: Insecure handling of incomplete and unvalidated financial data
Risk Level: MediumComplete vulnerable code snippet:
python def calc_equity(account: dict) -> float: cash = float(account.get("cash", 0)) positions = account.get("positions", []) market_value = 0.0 for pos in positions: qty = float(pos.get("qty", 0)) price = pos.get("currentPrice") if price is None: continue market_value += qty * float(price) return cash + market_value def main(): if len(sys.argv) < 2: print("Usage: update_account.py <account.json>", file=sys.stderr) sys.exit(1) path = Path(sys.argv[1]) account = load_json(path) equity = calc_equity(account) initial_cash = float(account.get("initialCash", equity or 0)) account["equity"] = round(equity, 2) account["totalReturnPct"] = round(((equity - initial_cash) / initial_cash) * 100, 4) if initial_cash else 0 account["updatedAt"] = datetime.now(timezone.utc).isoformat() dump_json(path, account)Technical Analysis
calc_equity()silently excludes every position whosecurrentPriceis absent. It then returns the partial valuation as if it were complete.main()persists that result to the authoritativeequityandtotalReturnPctfields without setting an incomplete-data status, preserving the prior valuation, or aborting the update.Consequently, a nonzero holding with no current price is implicitly valued at zero. The implementation also converts quantities and prices directly to floating-point values without checking whether they are finite or nonnegative. Special values such as
NaNorInfinity, if accepted by the parser and conversion path, can contaminate valuation output.This behavior conflicts with the Skill' ...[truncated 1831 chars]
- Remediation
View remediation
Remediation Suggestions
- Treat a missing price for any nonzero position as an incomplete valuation rather than assigning it zero value implicitly.
- Abort without overwriting
equityortotalReturnPct, or preserve the previous values and add an explicit status such asvaluationStatus: "incomplete". - Return structured calculation results containing the equity value, missing-price symbols, valuation timestamp, and completeness status.
- Validate that
cash,initialCash, quantities, and prices are numeric, finite, and consistent with the portfolio rules. Reject negative values unless explicitly supported. - Write updates atomically through a temporary file followed by replacement so validation or write failures do not partially corrupt account state.
- Make incomplete status visible in downstream reports and force
HOLDwhere reliable valuation is required. - Add tests covering missing prices, zero-quantity positions, malformed numeric fields,
NaN, infinity, negative values, and mixed complete/incomplete portfolios.
A hardened calculation should fail explicitly for a nonzero unpriced position, for example:
python import math def finite_nonnegative(value, field): number = float(value) if not math.isfinite(number) or number < 0: raise ValueError(f"{field} must be finite and nonnegative") return number def calc_equity(account): cash = finite_nonnegative(account.get("cash", 0), "cash") market_value = 0.0 missing = [] for pos in account.get("positions", []): qty = finite_nonnegative(pos.get("qty", 0), "position quantity") price = pos.get("currentPrice") if qty and price is None: missing.append(pos.get("ticker", "unknown")) continue if price is not None: market_value += qty * finite_nonnegative( price, "position currentPrice" ) if missing: ...[truncated 148 chars]