Commit 47b1d48
committed
feat(security): Phase 2 audit memory-safety sweep — RAII for buffers, EVP_PKEY*, EVP_PKEY_CTX, EVP_MD_CTX; shared_from_this in PQ async
Closes Phase 2 of plans/todo/security-audit.md across 24 C++ files
(+327/-480 net). All five Phase 2 items shipped together:
2.1 Raw `new uint8_t[]` → `std::unique_ptr<uint8_t[]>` + `release()` into
`NativeArrayBuffer` in Hash, HMAC, KMAC, BLAKE3, PBKDF2, Scrypt, HKDF,
the cipher base `update()`, ChaCha20/ChaCha20-Poly1305/XChaCha20-Poly1305/
XSalsa20-Poly1305, CCM `final()`, RSA-cipher decrypt sentinels, Ed25519
(6 sites), ML-DSA (3 sites), and ML-KEM (4 sites).
2.2 Raw `EVP_PKEY*` → `std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>`
in RSA, EC, and Ed25519 keypairs (DSA pattern as template).
`Ed25519::importPublicKey`/`importPrivateKey` now return owning
`EVP_PKEY_ptr` and use `EVP_PKEY_up_ref` for the borrow-the-instance-key
path, closing the audit-flagged leak at HybridEdKeyPair.cpp:155,221.
2.3 `Promise<…>::async([this, …])` → `auto self = this->shared_cast<…>();
[self, …]` in ML-DSA (3 sites) and ML-KEM (3 sites). Closes the
use-after-free risk if the JS object is GC'd while async is in flight.
2.4 Eliminate the unnecessary `EVP_PKEY_CTX_new_from_name` pre-creation in
Sign/Verify handles, ML-DSA, and Ed25519 — pass `nullptr` for the
`EVP_PKEY_CTX**` arg and let `EVP_DigestSignInit`/`EVP_DigestVerifyInit`
allocate from the key's keymgmt (matches `ncrypto::EVPMDCtxPointer::
signInit`). Crypto-specialist review confirmed the old code was actually
*leaking* the pre-allocated PKEY_CTX (OpenSSL silently overwrote the
pointer on success), so this fix closes both the audited double-free
*and* an unreported leak. Wraps EVP_MD_CTX/EVP_PKEY_CTX in local
`unique_ptr` aliases so all manual error-path frees collapse.
2.5 Replace Ed25519's two `ERR_error_string(ERR_get_error(), NULL)` calls
(thread-unsafe static buffer) with the shared `getOpenSSLError()` helper.
Defense-in-depth: `secureZero` added on Scrypt/HKDF error paths and on
Ed25519/ML-DSA/ML-KEM `getPrivateKey` BIO buffers.
Crypto-specialist approved all four substantive concerns: algorithm
selection unchanged (verified against OpenSSL 3.x m_sigver.c::do_sigver_init),
refcount semantics correct, BIO secure-zero is safe redundancy with
`BUF_MEM_free`'s `OPENSSL_clear_free`, `release()` + `make_shared` window
matches Nitro's own `ArrayBuffer::wrap`.1 parent 78a4f9e commit 47b1d48
25 files changed
Lines changed: 338 additions & 489 deletions
File tree
- packages/react-native-quick-crypto/cpp
- blake3
- cipher
- ec
- ed25519
- hash
- hkdf
- hmac
- kmac
- mldsa
- mlkem
- pbkdf2
- rsa
- scrypt
- sign
- plans/todo
Lines changed: 5 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
67 | 68 | | |
68 | 69 | | |
69 | 70 | | |
70 | | - | |
71 | | - | |
| 71 | + | |
| 72 | + | |
72 | 73 | | |
73 | | - | |
| 74 | + | |
| 75 | + | |
74 | 76 | | |
75 | 77 | | |
76 | 78 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
110 | 110 | | |
111 | 111 | | |
112 | 112 | | |
113 | | - | |
114 | | - | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
115 | 116 | | |
116 | 117 | | |
117 | 118 | | |
| |||
Lines changed: 7 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | | - | |
| 71 | + | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
75 | | - | |
| 74 | + | |
76 | 75 | | |
77 | 76 | | |
78 | 77 | | |
79 | 78 | | |
80 | 79 | | |
81 | 80 | | |
82 | 81 | | |
83 | | - | |
| 82 | + | |
| 83 | + | |
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | | - | |
91 | | - | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
92 | 93 | | |
93 | 94 | | |
94 | 95 | | |
Lines changed: 8 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
76 | | - | |
| 75 | + | |
77 | 76 | | |
78 | 77 | | |
79 | 78 | | |
80 | 79 | | |
81 | 80 | | |
82 | 81 | | |
83 | 82 | | |
84 | | - | |
| 83 | + | |
| 84 | + | |
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
93 | | - | |
| 93 | + | |
94 | 94 | | |
95 | | - | |
96 | | - | |
| 95 | + | |
97 | 96 | | |
98 | 97 | | |
99 | 98 | | |
100 | 99 | | |
101 | 100 | | |
102 | 101 | | |
103 | 102 | | |
104 | | - | |
| 103 | + | |
| 104 | + | |
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
| |||
Lines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
109 | | - | |
| 109 | + | |
110 | 110 | | |
111 | 111 | | |
112 | | - | |
| 112 | + | |
113 | 113 | | |
114 | 114 | | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
119 | 118 | | |
120 | 119 | | |
121 | 120 | | |
122 | 121 | | |
123 | | - | |
| 122 | + | |
| 123 | + | |
124 | 124 | | |
125 | 125 | | |
126 | 126 | | |
| |||
Lines changed: 6 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
283 | 283 | | |
284 | 284 | | |
285 | 285 | | |
286 | | - | |
287 | | - | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
288 | 289 | | |
289 | 290 | | |
290 | 291 | | |
| |||
306 | 307 | | |
307 | 308 | | |
308 | 309 | | |
309 | | - | |
310 | | - | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
311 | 313 | | |
312 | 314 | | |
313 | 315 | | |
| |||
Lines changed: 14 additions & 13 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
73 | | - | |
| 73 | + | |
74 | 74 | | |
75 | 75 | | |
76 | | - | |
| 76 | + | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
81 | | - | |
| 80 | + | |
82 | 81 | | |
83 | 82 | | |
84 | 83 | | |
85 | 84 | | |
86 | 85 | | |
87 | | - | |
| 86 | + | |
| 87 | + | |
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
94 | | - | |
| 94 | + | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
102 | | - | |
| 101 | + | |
103 | 102 | | |
104 | 103 | | |
105 | 104 | | |
106 | 105 | | |
107 | 106 | | |
108 | | - | |
| 107 | + | |
| 108 | + | |
109 | 109 | | |
110 | 110 | | |
111 | 111 | | |
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
136 | | - | |
137 | | - | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
138 | 139 | | |
139 | 140 | | |
140 | 141 | | |
| |||
Lines changed: 14 additions & 13 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
63 | | - | |
| 63 | + | |
64 | 64 | | |
65 | | - | |
| 65 | + | |
66 | 66 | | |
67 | 67 | | |
68 | | - | |
69 | | - | |
| 68 | + | |
70 | 69 | | |
71 | 70 | | |
72 | 71 | | |
73 | 72 | | |
74 | 73 | | |
75 | | - | |
| 74 | + | |
| 75 | + | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
| 82 | + | |
83 | 83 | | |
84 | | - | |
| 84 | + | |
85 | 85 | | |
86 | 86 | | |
87 | | - | |
88 | | - | |
| 87 | + | |
89 | 88 | | |
90 | 89 | | |
91 | 90 | | |
92 | 91 | | |
93 | 92 | | |
94 | | - | |
| 93 | + | |
| 94 | + | |
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
115 | | - | |
116 | | - | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
117 | 118 | | |
118 | 119 | | |
119 | 120 | | |
| |||
0 commit comments