V12 issues - #126
Open
illuzen wants to merge 39 commits into
Open
Conversation
Authority secrets on --mnemonic/--seed were visible in process argv, shell history, and CI logs. Always read them from a hidden prompt instead. Co-authored-by: Cursor <cursoragent@cursor.com>
A single corrupt or non-canonical wallet JSON aborted the entire list. Validate Quantus SS58 addresses at load and isolate per-file failures. Co-authored-by: Cursor <cursoragent@cursor.com>
Taking the first same-block MultisigCreated event could report another transaction's address. Match creator, signers, threshold, and nonce. Co-authored-by: Cursor <cursoragent@cursor.com>
Passwords on --password/-p were visible in process argv and logs. Reject them at the shared helper and wallet-create boundary. Co-authored-by: Cursor <cursoragent@cursor.com>
check_execution_success treated a missing extrinsic hash as success. Return a NetworkError so callers do not report false confirmations. Co-authored-by: Cursor <cursoragent@cursor.com>
Wallet directories and files inherited umask defaults, allowing local users to read ciphertext and KDF metadata. Set dir 0700 and files 0600. Co-authored-by: Cursor <cursoragent@cursor.com>
--password-file accepted world-readable files. On Unix, require a regular file owned by the caller with no group/other access bits. Co-authored-by: Cursor <cursoragent@cursor.com>
User-facing batch transfers used non-atomic utility.batch while docs promised fail-all semantics. Switch the builder to batch_all. Co-authored-by: Cursor <cursoragent@cursor.com>
Generating an ephemeral mnemonic when a wallet had none could strand funds at irrecoverable addresses. Error instead and require a mnemonic. Co-authored-by: Cursor <cursoragent@cursor.com>
Error-substring retries re-signed with an incremented nonce and could duplicate extrinsics. Submit once with a fresh nonce and surface Subxt errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Envelope address was trusted without keypair binding, enabling transfer redirect and spoofed listings. Validate on decrypt, stop passwordless trust of the envelope, and propagate legacy migration save failures. Co-authored-by: Cursor <cursoragent@cursor.com>
Predictable temp paths and check-then-write overwrites allowed races and wallet replacement. Use exclusive create, safer temps, name checks, and locks. Co-authored-by: Cursor <cursoragent@cursor.com>
RPC connections trusted any node metadata for signing context. Require spec name quantus and a compatible runtime version before proceeding. Co-authored-by: Cursor <cursoragent@cursor.com>
Missing or out-of-range tokenDecimals/symbol/ss58Format silently mis-scaled amounts. Validate properties before using them for formatting. Co-authored-by: Cursor <cursoragent@cursor.com>
Destination-only matching could select another same-block transfer. Require a unique match on from, amount, and transfer_count. Co-authored-by: Cursor <cursoragent@cursor.com>
Self-update applied GitHub archives without checking published sha256sums. Download the sibling checksum file and verify before replace. Co-authored-by: Cursor <cursoragent@cursor.com>
Legacy Argon2 digests must not be rewritten to disk. Encrypt already strips digests; save paths now reject any remaining embedded key material. Co-authored-by: Cursor <cursoragent@cursor.com>
Address derivation previously panicked on bad Dilithium public key bytes. Propagate InvalidPublicKey through wallet creation and views. Co-authored-by: Cursor <cursoragent@cursor.com>
Node URLs with userinfo were logged and embedded in errors. Sanitize diagnostics so passwords are not disclosed. Co-authored-by: Cursor <cursoragent@cursor.com>
Secrets on --secret were visible in process argv; require --secret-file. Also treat ExtrinsicFailed as dominant so failed txs are not verified. Co-authored-by: Cursor <cursoragent@cursor.com>
Printing mnemonics to stdout risked shoulder-surfing and log capture. Require --output and write an owner-only file instead. Co-authored-by: Cursor <cursoragent@cursor.com>
Duplicate signers inflated predicted addresses and could satisfy thresholds incorrectly. Sort and dedup before prediction and checks. Co-authored-by: Cursor <cursoragent@cursor.com>
Checked count accumulation and reject non-advancing key cursors so malicious RPC pages cannot loop or wrap the entry count. Co-authored-by: Cursor <cursoragent@cursor.com>
Unbounded tx_progress waits could hang forever. Apply inactivity and overall deadlines and surface stream timeouts. Co-authored-by: Cursor <cursoragent@cursor.com>
Untrusted Subsquid amounts were summed with wrapping u128 +=. Reject overflows instead of silently wrapping totals. Co-authored-by: Cursor <cursoragent@cursor.com>
Batch sizing used a soft heuristic that could exceed the runtime call count limit. Read Utility::batched_calls_limit and fail closed. Co-authored-by: Cursor <cursoragent@cursor.com>
Require aggregate counts, offset-paginate single over-limit blocks, apply caller offset once globally, and exclude spent nullifiers from pending sets. Co-authored-by: Cursor <cursoragent@cursor.com>
Proof inputs retained secret bytes after generation. Clear them before returning so secrets do not linger in process memory. Co-authored-by: Cursor <cursoragent@cursor.com>
Reject oversized/mismatched ZK Merkle proofs and read recursive flow state from finalized blocks instead of best-block tips. Co-authored-by: Cursor <cursoragent@cursor.com>
Key material and plaintext buffers were retained after use. Clear them on Drop and after crypto operations; redact Debug output. Co-authored-by: Cursor <cursoragent@cursor.com>
Harden decimal formatting, transfer display, collective remove rank, delay encoding, and public helpers that previously panicked on bad input. Co-authored-by: Cursor <cursoragent@cursor.com>
Load circuit bundles only after manifest hash checks, refuse symlink redirection, and publish generated-bins via atomic directory replace. Co-authored-by: Cursor <cursoragent@cursor.com>
Cap block-list and storage-iterate limits, surface missing extrinsic and nonce absence, reject bad JSON numerics and duplicate multisend recipients, and use checked metadata counters. Co-authored-by: Cursor <cursoragent@cursor.com>
Create previously fell through to an empty password after rejecting --password. Obtain a new password via file, env, or confirmed prompt, and require --allow-empty-password for empty development wallets. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace deprecated GenericArray::from_slice usage, simplify char/find and Option helpers, const-assert tx timeouts, allow intentional public SDK dead_code, and stop moving out of Drop WalletData in examples. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3d781aa. Configure here.
| const MILLIS_PER_SECOND: u64 = 1_000; | ||
| const TX_STATUS_INACTIVITY_TIMEOUT_SECS: u64 = 30; | ||
| const TX_STATUS_INCLUDED_TIMEOUT_SECS: u64 = 5 * 60; | ||
| const TX_STATUS_FINALIZED_TIMEOUT_SECS: u64 = 30 * 60; |
There was a problem hiding this comment.
Inactivity timeout aborts finalization waits
High Severity
The new status watcher applies a fixed 30s inactivity timeout on every tx_progress.next() wait, including after best-block inclusion while waiting for finalization. On Quantus PoW, finalization often has silent gaps longer than 30s, so --finalized and other finalized waits can fail even though the overall 30-minute deadline has not elapsed. The error text also always cites the 30s inactivity window, including when the overall watch deadline expires.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3d781aa. Configure here.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
After best-block inclusion, wait only on the overall watch deadline so PoW finality gaps longer than 30s don't fail --finalized. Distinguish inactivity vs overall-deadline timeout errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses the V12 security audit findings in
v12-issues.md.Scope: only findings with
Validity: Unreviewed. V12 marks 314 Low findings asValidity: Invalid(likely incorrect); those are excluded from this analysis and are not treated as open work.This PR remediates all 20 High and 30 Medium Unreviewed findings (with red→green tests where applicable). The 11 Unreviewed Lows are listed below; most remain open for follow-up. A few Invalid Lows were hardened opportunistically and are noted separately (out of audit scope).
34 commits on
illuzen/v12-2. Library suite: 211+ tests passing.High (20/20 addressed)
--passwordCLI credentials0700, files0600utility.batch_alle0be480); residual: refuse re-persisting digestsMedium (30/30 addressed)
--output(0o600)--secretargv--secret-fileInvalidPublicKeychecked_pow+ validation--min-rankmanifest.jsonSHA-256checked_addbatched_calls_limitLow (11 Unreviewed — in scope)
Excluded: 314 Low with
Validity: InvalidOut of scope per V12. No further triage required for merge of this PR.
Opportunistic hardening (Invalid Lows — not audit blockers)
Some Invalid Lows were still tightened while adjacent to High/Medium work (e.g. block-list bounds, storage iterate cap, JSON numeric parsing, multisend dupes, metadata
checked_add). These are optional defense-in-depth, not required to close the Unreviewed set.Info (2 Unreviewed)
Breaking / UX changes callers should know
wallet import/from-seed: no--mnemonic/--seedflags (stdin prompts)--password/-prejected everywhere; use--password-file, env, or promptwallet createno longer silently uses an empty password; prompts (with confirm),--password-file, or env; empty only via--allow-empty-password--secret→--secret-filewallet export: requires--outputfile (no stdout mnemonic dump)--min-rankgenerated-bins/is a real directory withmanifest.json(symlink-style bins rejected)QuantusClient::newrejects non-Quantus / incompatible runtimesbatch_all(atomic fail-all)Test plan
cargo test --lib(211 passed)quantus wallet import --name x --mnemonic '...'fails clap parsequantus wallet create --name x --password secreterrors with guidance0700/0600after createspecNameRPC failsquantus updaterefuses checksum mismatch (if exercising updater)SKIP_CIRCUIT_BUILDonce for newgenerated-binslayoutCommits