fix: replace weak RandomState PRNG with OS-backed getrandom crate#1
fix: replace weak RandomState PRNG with OS-backed getrandom crate#1
Conversation
The previous getrandom implementation used std's RandomState/SipHash which is not cryptographically secure. Guests relying on api_random for key generation or nonces were vulnerable. Switch to the getrandom crate which uses the OS CSPRNG (/dev/urandom, BCryptGenRandom, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA dependency on the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@oxide-browser/src/capabilities.rs`:
- Line 1328: Replace the panic-based RNG call with Result handling: change the
direct ::getrandom::getrandom(buf).expect(...) invocation inside api_random to a
fallible wrapper (e.g., a new getrandom function returning Result<(), String> or
map_err inline) and update api_random to handle the Err case instead of
panicking—log the error and return the appropriate failure sentinel (or
propagate a trap via the caller) rather than calling expect. Locate the
getrandom invocation and the api_random host callback to implement the map_err()
conversion and the subsequent error branch that avoids terminating the process.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f199cf38-21fe-4c8e-9799-5bc7e35e1c1f
📒 Files selected for processing (2)
oxide-browser/Cargo.tomloxide-browser/src/capabilities.rs
The previous getrandom implementation used std's RandomState/SipHash which is not cryptographically secure. Guests relying on api_random for key generation or nonces were vulnerable. Switch to the getrandom crate which uses the OS CSPRNG (/dev/urandom, BCryptGenRandom, etc.).
Summary by CodeRabbit
Refactor
Chores