Skip to content

Comments

Harden seed wipe, secret handling, and command execution paths#272

Open
3rdIteration wants to merge 1 commit intodevfrom
codex/fix-security-vulnerabilities-in-seedsigner-ejpoef
Open

Harden seed wipe, secret handling, and command execution paths#272
3rdIteration wants to merge 1 commit intodevfrom
codex/fix-security-vulnerabilities-in-seedsigner-ejpoef

Conversation

@3rdIteration
Copy link
Owner

Motivation

  • Reduce secret leakage and improve memory-hygiene by ensuring auto-wipe actively zeroizes in-memory secrets (loaded seeds, cached PINs, entropy caches, and encrypted-QR keys) rather than merely dropping references.
  • Eliminate places that embed raw secret bytes into UI/log strings and reduce command-injection surface by removing string-assembled commands and os.system uses.

Description

  • Actively wipe all loaded Seed objects during inactivity auto-wipe by calling seed.wipe() before clearing storage.seeds, and zeroize controller-level secret fields such as cached PINs and password entropy cache via secure-delete helpers. (src/seedsigner/controller.py)
  • Add wipe() semantics to EncryptedQR and ensure EncryptedQRStorage.clear_encryptedqr() zeroizes the encryption key before dropping the object. (src/seedsigner/models/encryptedqr.py)
  • Sanitize SeedKeeper fallback messages to avoid including raw entropy/passphrase hex in UI text and zeroize temporary parsed buffers on success/failure paths. (src/seedsigner/views/tools_views.py)
  • Replace string-based command assembly / shlex.split in run_globalplatform() with explicit argument lists and update callers to pass list[str] argv arrays, preventing ambiguous splitting/parsing. (src/seedsigner/helpers/seedkeeper_utils.py, src/seedsigner/views/tools_views.py)
  • Replace targeted os.system(...) uses with subprocess.run([...], shell=False) via a small helper _run_cmd() that optionally prefixes sudo when needed, reducing runtime injection risk in settings and tool flows. (src/seedsigner/models/settings.py, src/seedsigner/views/tools_views.py)
  • Add a controller-level helper to zeroize cached entropy/roll buffers and update password-entropy cache helpers to wipe stored buffers. (src/seedsigner/controller.py, src/seedsigner/views/tools_views.py)
  • Updated and added focused unit tests to cover wipe-timeout behavior and adjusted tests that relied on prior command string expectations. (tests/test_controller.py, tests/test_seedkeeper_install.py, tests/test_settings.py)

Testing

  • Ran the focused test set: pytest -q tests/test_controller.py tests/test_seedkeeper_install.py tests/test_settings.py, all tests passed (20 passed).
  • Verified targeted behavioral changes with grep checks for removed os.system and shlex.split usages in updated files and validated callers were updated to use argv lists.
  • No new crypto primitives or external dependencies were introduced; secure-delete helpers (existing) were reused for zeroization.

Codex Task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant