Commit 73ea9f3
committed
feat(security): own EVP_CIPHER_CTX via unique_ptr in HybridCipher base
Three subclasses (CCMCipher, ChaCha20Cipher, ChaCha20Poly1305Cipher)
implemented destructors as `~SubClass() { ctx = nullptr; }` — they
nulled the parent's raw pointer without freeing it first, expecting
the parent destructor to do the cleanup. The parent then saw `ctx ==
nullptr` and skipped the EVP_CIPHER_CTX_free call entirely. Result:
every cipher object of those three types leaked its OpenSSL context.
Move ownership into the base class with a unique_ptr that has the
EVP_CIPHER_CTX_free deleter baked in. Subclasses can no longer
mismanage ctx because the destruction order (subclass → base) makes
the parent's unique_ptr destructor run automatically and free the
context exactly once. The buggy three subclass destructors are
removed (defaulted), and XSalsa20Cipher's destructor drops its now-
redundant `ctx = nullptr` line while keeping the secureZero calls
from Phase 1.2.
Internally every `EVP_*Foo*(ctx, ...)` OpenSSL call now uses
`ctx.get()`; every `ctx = EVP_CIPHER_CTX_new()` becomes
`ctx.reset(EVP_CIPHER_CTX_new())`; every manual cleanup of
`EVP_CIPHER_CTX_free(ctx); ctx = nullptr;` collapses to `ctx.reset()`.
Touched files: HybridCipher, CCMCipher, ChaCha20Cipher,
ChaCha20Poly1305Cipher, GCMCipher, OCBCipher, XSalsa20Cipher.
No new tests — the existing AEAD round-trip tests exercise both the
constructor (ctx.reset(EVP_CIPHER_CTX_new())) and the destructor
(unique_ptr cleanup) in every test that uses these ciphers.
Phase 1.3 of plans/todo/security-audit.md.1 parent 2fae6bd commit 73ea9f3
11 files changed
Lines changed: 86 additions & 107 deletions
File tree
- packages/react-native-quick-crypto/cpp/cipher
Lines changed: 10 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
| 47 | + | |
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| |||
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
69 | | - | |
| 69 | + | |
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
| 82 | + | |
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
| |||
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
| 118 | + | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
125 | | - | |
| 125 | + | |
126 | 126 | | |
127 | 127 | | |
128 | 128 | | |
| |||
133 | 133 | | |
134 | 134 | | |
135 | 135 | | |
136 | | - | |
| 136 | + | |
137 | 137 | | |
138 | 138 | | |
139 | 139 | | |
| |||
179 | 179 | | |
180 | 180 | | |
181 | 181 | | |
182 | | - | |
| 182 | + | |
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
| |||
190 | 190 | | |
191 | 191 | | |
192 | 192 | | |
193 | | - | |
| 193 | + | |
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
| 10 | + | |
| 11 | + | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
Lines changed: 8 additions & 13 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
| 10 | + | |
| 11 | + | |
15 | 12 | | |
16 | 13 | | |
17 | 14 | | |
| |||
20 | 17 | | |
21 | 18 | | |
22 | 19 | | |
23 | | - | |
| 20 | + | |
24 | 21 | | |
25 | 22 | | |
26 | 23 | | |
27 | 24 | | |
28 | 25 | | |
29 | | - | |
| 26 | + | |
30 | 27 | | |
31 | 28 | | |
32 | 29 | | |
33 | | - | |
34 | | - | |
| 30 | + | |
35 | 31 | | |
36 | 32 | | |
37 | 33 | | |
| |||
52 | 48 | | |
53 | 49 | | |
54 | 50 | | |
55 | | - | |
| 51 | + | |
56 | 52 | | |
57 | 53 | | |
58 | 54 | | |
59 | | - | |
60 | | - | |
| 55 | + | |
61 | 56 | | |
62 | 57 | | |
63 | 58 | | |
| |||
76 | 71 | | |
77 | 72 | | |
78 | 73 | | |
79 | | - | |
| 74 | + | |
80 | 75 | | |
81 | 76 | | |
82 | 77 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
| 10 | + | |
| 11 | + | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
Lines changed: 12 additions & 17 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
| 10 | + | |
| 11 | + | |
15 | 12 | | |
16 | 13 | | |
17 | 14 | | |
| |||
20 | 17 | | |
21 | 18 | | |
22 | 19 | | |
23 | | - | |
| 20 | + | |
24 | 21 | | |
25 | 22 | | |
26 | 23 | | |
27 | 24 | | |
28 | 25 | | |
29 | | - | |
| 26 | + | |
30 | 27 | | |
31 | 28 | | |
32 | 29 | | |
33 | | - | |
34 | | - | |
| 30 | + | |
35 | 31 | | |
36 | 32 | | |
37 | 33 | | |
| |||
52 | 48 | | |
53 | 49 | | |
54 | 50 | | |
55 | | - | |
| 51 | + | |
56 | 52 | | |
57 | 53 | | |
58 | 54 | | |
59 | | - | |
60 | | - | |
| 55 | + | |
61 | 56 | | |
62 | 57 | | |
63 | 58 | | |
| |||
77 | 72 | | |
78 | 73 | | |
79 | 74 | | |
80 | | - | |
| 75 | + | |
81 | 76 | | |
82 | 77 | | |
83 | 78 | | |
| |||
97 | 92 | | |
98 | 93 | | |
99 | 94 | | |
100 | | - | |
| 95 | + | |
101 | 96 | | |
102 | 97 | | |
103 | 98 | | |
| |||
116 | 111 | | |
117 | 112 | | |
118 | 113 | | |
119 | | - | |
| 114 | + | |
120 | 115 | | |
121 | 116 | | |
122 | 117 | | |
| |||
136 | 131 | | |
137 | 132 | | |
138 | 133 | | |
139 | | - | |
| 134 | + | |
140 | 135 | | |
141 | 136 | | |
142 | 137 | | |
| |||
158 | 153 | | |
159 | 154 | | |
160 | 155 | | |
161 | | - | |
| 156 | + | |
162 | 157 | | |
163 | 158 | | |
164 | 159 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
| 10 | + | |
| 11 | + | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
Lines changed: 9 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
| 10 | + | |
| 11 | + | |
15 | 12 | | |
16 | 13 | | |
17 | 14 | | |
| |||
20 | 17 | | |
21 | 18 | | |
22 | 19 | | |
23 | | - | |
| 20 | + | |
24 | 21 | | |
25 | 22 | | |
26 | 23 | | |
27 | 24 | | |
28 | 25 | | |
29 | | - | |
| 26 | + | |
30 | 27 | | |
31 | 28 | | |
32 | 29 | | |
33 | | - | |
34 | | - | |
| 30 | + | |
35 | 31 | | |
36 | 32 | | |
37 | 33 | | |
| |||
40 | 36 | | |
41 | 37 | | |
42 | 38 | | |
43 | | - | |
| 39 | + | |
44 | 40 | | |
45 | 41 | | |
46 | 42 | | |
47 | | - | |
48 | | - | |
| 43 | + | |
49 | 44 | | |
50 | 45 | | |
51 | 46 | | |
| |||
55 | 50 | | |
56 | 51 | | |
57 | 52 | | |
58 | | - | |
| 53 | + | |
59 | 54 | | |
60 | 55 | | |
61 | 56 | | |
62 | | - | |
63 | | - | |
| 57 | + | |
64 | 58 | | |
65 | 59 | | |
66 | 60 | | |
| |||
0 commit comments