[PM-8275] Implement a zeroizing allocator #788
Conversation
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #788 +/- ##
=======================================
Coverage ? 57.96%
=======================================
Files ? 179
Lines ? 11652
Branches ? 0
=======================================
Hits ? 6754
Misses ? 4898
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Hinton
changed the title
Hinton
changed the title
There was a problem hiding this comment.
Allocator look good to me and seems to be working correctly, but I'm not sure about having to define #[global_allocator] in every crate. It might make us accidentally forget to enable it in some future new crate. We should also probably enable it for bitwarden-uniffi.
What if instead we defined the global allocator in the bitwarden crate under some secure-allocator feature flag which is enabled by default? That way all our crates will get the new allocator for free, and anyone using our Rust crate who might prefer a different allocator can just disable the default flags.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
crates/bitwarden-crypto/src/lib.rs
Outdated
There was a problem hiding this comment.
Please add features documentation to this front page as described in https://doc.rust-lang.org/cargo/reference/features.html#feature-documentation-and-discovery.
Especially since you're worried about unintentionally disabling memory hardening and thus not following the additive feature standard
There was a problem hiding this comment.
Good point. I've added some documentation.
# Conflicts: # crates/memory-testing/src/main.rs
Type of change
Objective
Add a custom allocator which zerorizes memory on drop. This should ensure everything heap allocated gets sanitized before deallocated, and avoid sensitive memory being exposed more reliably than using
Sensitive.Before you submit