Skip to content

Commit e1b306a

Browse files
booradboorad
authored
feat(security): Phase 2 audit memory-safety sweep (#984)
1 parent 78a4f9e commit e1b306a

27 files changed

Lines changed: 358 additions & 501 deletions

‎.github/workflows/e2e-android-test.yml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ on:
1010
paths:
1111
- '.github/workflows/e2e-android-test.yml'
1212
- 'example/**'
13-
- 'cpp/**'
14-
- 'nitrogen/**'
15-
- 'src/**'
13+
- 'packages/react-native-quick-crypto/cpp/**'
14+
- 'packages/react-native-quick-crypto/nitrogen/**'
15+
- 'packages/react-native-quick-crypto/src/**'
1616
- 'packages/react-native-quick-crypto/android/**'
1717
push:
1818
branches: [main]
1919
paths:
2020
- 'example/**'
21-
- 'cpp/**'
22-
- 'nitrogen/**'
23-
- 'src/**'
21+
- 'packages/react-native-quick-crypto/cpp/**'
22+
- 'packages/react-native-quick-crypto/nitrogen/**'
23+
- 'packages/react-native-quick-crypto/src/**'
2424
- 'packages/react-native-quick-crypto/android/**'
2525

2626
env:

‎.github/workflows/e2e-ios-test.yml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ on:
1010
paths:
1111
- '.github/workflows/e2e-ios-test.yml'
1212
- 'example/**'
13-
- 'cpp/**'
14-
- 'nitrogen/**'
15-
- 'src/**'
13+
- 'packages/react-native-quick-crypto/cpp/**'
14+
- 'packages/react-native-quick-crypto/nitrogen/**'
15+
- 'packages/react-native-quick-crypto/src/**'
1616
- 'packages/react-native-quick-crypto/ios/**'
1717
push:
1818
branches: [main]
1919
paths:
2020
- 'example/**'
21-
- 'cpp/**'
22-
- 'nitrogen/**'
23-
- 'src/**'
21+
- 'packages/react-native-quick-crypto/cpp/**'
22+
- 'packages/react-native-quick-crypto/nitrogen/**'
23+
- 'packages/react-native-quick-crypto/src/**'
2424
- 'packages/react-native-quick-crypto/ios/**'
2525

2626
jobs:

‎packages/react-native-quick-crypto/cpp/blake3/HybridBlake3.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <NitroModules/ArrayBuffer.hpp>
44
#include <cstring>
5+
#include <memory>
56
#include <stdexcept>
67

78
#include "QuickCryptoUtils.hpp"
@@ -67,10 +68,11 @@ std::shared_ptr<ArrayBuffer> HybridBlake3::digest(std::optional<double> length)
6768
outLen = static_cast<size_t>(len);
6869
}
6970

70-
auto output = new uint8_t[outLen];
71-
blake3_hasher_finalize(&hasher, output, outLen);
71+
auto output = std::make_unique<uint8_t[]>(outLen);
72+
blake3_hasher_finalize(&hasher, output.get(), outLen);
7273

73-
return std::make_shared<margelo::nitro::NativeArrayBuffer>(output, outLen, [=]() { delete[] output; });
74+
uint8_t* raw_ptr = output.get();
75+
return std::make_shared<margelo::nitro::NativeArrayBuffer>(output.release(), outLen, [raw_ptr]() { delete[] raw_ptr; });
7476
}
7577

7678
void HybridBlake3::reset() {

‎packages/react-native-quick-crypto/cpp/cipher/CCMCipher.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,9 @@ std::shared_ptr<ArrayBuffer> CCMCipher::final() {
110110
// CCM decryption does not use final. Verification happens in the last update call.
111111
if (!is_cipher) {
112112
is_finalized = true;
113-
unsigned char* empty_output = new unsigned char[0];
114-
return std::make_shared<NativeArrayBuffer>(empty_output, 0, [=]() { delete[] empty_output; });
113+
auto empty_output = std::make_unique<unsigned char[]>(0);
114+
unsigned char* raw_ptr = empty_output.get();
115+
return std::make_shared<NativeArrayBuffer>(empty_output.release(), 0, [raw_ptr]() { delete[] raw_ptr; });
115116
}
116117

117118
// Proceed only for encryption

‎packages/react-native-quick-crypto/cpp/cipher/ChaCha20Cipher.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,27 +68,28 @@ std::shared_ptr<ArrayBuffer> ChaCha20Cipher::update(const std::shared_ptr<ArrayB
6868

6969
// For ChaCha20, output size equals input size since it's a stream cipher
7070
int out_len = in_len;
71-
uint8_t* out = new uint8_t[out_len];
71+
auto out_buf = std::make_unique<uint8_t[]>(out_len);
7272

7373
// Perform the cipher update operation
74-
if (EVP_CipherUpdate(ctx.get(), out, &out_len, native_data->data(), in_len) != 1) {
75-
delete[] out;
74+
if (EVP_CipherUpdate(ctx.get(), out_buf.get(), &out_len, native_data->data(), in_len) != 1) {
7675
unsigned long err = ERR_get_error();
7776
char err_buf[256];
7877
ERR_error_string_n(err, err_buf, sizeof(err_buf));
7978
throw std::runtime_error("ChaCha20Cipher: Failed to update: " + std::string(err_buf));
8079
}
8180

8281
// Create and return a new buffer of exact size needed
83-
return std::make_shared<NativeArrayBuffer>(out, out_len, [=]() { delete[] out; });
82+
uint8_t* raw_ptr = out_buf.get();
83+
return std::make_shared<NativeArrayBuffer>(out_buf.release(), out_len, [raw_ptr]() { delete[] raw_ptr; });
8484
}
8585

8686
std::shared_ptr<ArrayBuffer> ChaCha20Cipher::final() {
8787
checkCtx();
8888
checkNotFinalized();
8989
is_finalized = true;
90-
unsigned char* empty_output = new unsigned char[0];
91-
return std::make_shared<NativeArrayBuffer>(empty_output, 0, [=]() { delete[] empty_output; });
90+
auto empty_buf = std::make_unique<unsigned char[]>(0);
91+
unsigned char* raw_ptr = empty_buf.get();
92+
return std::make_shared<NativeArrayBuffer>(empty_buf.release(), 0, [raw_ptr]() { delete[] raw_ptr; });
9293
}
9394

9495
} // namespace margelo::nitro::crypto

‎packages/react-native-quick-crypto/cpp/cipher/ChaCha20Poly1305Cipher.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,19 @@ std::shared_ptr<ArrayBuffer> ChaCha20Poly1305Cipher::update(const std::shared_pt
6969

7070
// For ChaCha20-Poly1305, output size equals input size since it's a stream cipher
7171
int out_len = in_len;
72-
uint8_t* out = new uint8_t[out_len];
72+
auto out_buf = std::make_unique<uint8_t[]>(out_len);
7373

7474
// Perform the cipher update operation
75-
if (EVP_CipherUpdate(ctx.get(), out, &out_len, native_data->data(), in_len) != 1) {
76-
delete[] out;
75+
if (EVP_CipherUpdate(ctx.get(), out_buf.get(), &out_len, native_data->data(), in_len) != 1) {
7776
unsigned long err = ERR_get_error();
7877
char err_buf[256];
7978
ERR_error_string_n(err, err_buf, sizeof(err_buf));
8079
throw std::runtime_error("ChaCha20Poly1305Cipher: Failed to update: " + std::string(err_buf));
8180
}
8281

8382
// Create and return a new buffer of exact size needed
84-
return std::make_shared<NativeArrayBuffer>(out, out_len, [=]() { delete[] out; });
83+
uint8_t* raw_ptr = out_buf.get();
84+
return std::make_shared<NativeArrayBuffer>(out_buf.release(), out_len, [raw_ptr]() { delete[] raw_ptr; });
8585
}
8686

8787
std::shared_ptr<ArrayBuffer> ChaCha20Poly1305Cipher::final() {
@@ -90,18 +90,18 @@ std::shared_ptr<ArrayBuffer> ChaCha20Poly1305Cipher::final() {
9090

9191
// For ChaCha20-Poly1305, we need to call final to generate the tag
9292
int out_len = 0;
93-
unsigned char* out = new unsigned char[0];
93+
auto out_buf = std::make_unique<unsigned char[]>(0);
9494

95-
if (EVP_CipherFinal_ex(ctx.get(), out, &out_len) != 1) {
96-
delete[] out;
95+
if (EVP_CipherFinal_ex(ctx.get(), out_buf.get(), &out_len) != 1) {
9796
unsigned long err = ERR_get_error();
9897
char err_buf[256];
9998
ERR_error_string_n(err, err_buf, sizeof(err_buf));
10099
throw std::runtime_error("ChaCha20Poly1305Cipher: Failed to finalize: " + std::string(err_buf));
101100
}
102101

103102
is_finalized = true;
104-
return std::make_shared<NativeArrayBuffer>(out, out_len, [=]() { delete[] out; });
103+
unsigned char* raw_ptr = out_buf.get();
104+
return std::make_shared<NativeArrayBuffer>(out_buf.release(), out_len, [raw_ptr]() { delete[] raw_ptr; });
105105
}
106106

107107
bool ChaCha20Poly1305Cipher::setAAD(const std::shared_ptr<ArrayBuffer>& data, std::optional<double> plaintextLength) {

‎packages/react-native-quick-crypto/cpp/cipher/HybridCipher.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,21 +106,21 @@ std::shared_ptr<ArrayBuffer> HybridCipher::update(const std::shared_ptr<ArrayBuf
106106
}
107107

108108
int out_len = in_len + EVP_CIPHER_CTX_block_size(ctx.get());
109-
uint8_t* out = new uint8_t[out_len];
109+
auto out_buf = std::make_unique<uint8_t[]>(out_len);
110110
// Perform the cipher update operation. The real size of the output is
111111
// returned in out_len
112-
int ret = EVP_CipherUpdate(ctx.get(), out, &out_len, native_data->data(), in_len);
112+
int ret = EVP_CipherUpdate(ctx.get(), out_buf.get(), &out_len, native_data->data(), in_len);
113113

114114
if (!ret) {
115115
unsigned long err = ERR_get_error();
116116
char err_buf[256];
117117
ERR_error_string_n(err, err_buf, sizeof(err_buf));
118-
delete[] out;
119118
throw std::runtime_error("Cipher update failed: " + std::string(err_buf));
120119
}
121120

122121
// Create and return a new buffer of exact size needed
123-
return std::make_shared<NativeArrayBuffer>(out, out_len, [=]() { delete[] out; });
122+
uint8_t* raw_ptr = out_buf.get();
123+
return std::make_shared<NativeArrayBuffer>(out_buf.release(), out_len, [raw_ptr]() { delete[] raw_ptr; });
124124
}
125125

126126
std::shared_ptr<ArrayBuffer> HybridCipher::final() {

‎packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,9 @@ std::shared_ptr<ArrayBuffer> HybridRsaCipher::publicDecrypt(const std::shared_pt
283283

284284
if (outlen == 0) {
285285
EVP_PKEY_CTX_free(ctx);
286-
uint8_t* empty_buf = new uint8_t[1];
287-
return std::make_shared<NativeArrayBuffer>(empty_buf, 0, [empty_buf]() { delete[] empty_buf; });
286+
auto empty_buf = std::make_unique<uint8_t[]>(1);
287+
uint8_t* raw_ptr = empty_buf.get();
288+
return std::make_shared<NativeArrayBuffer>(empty_buf.release(), 0, [raw_ptr]() { delete[] raw_ptr; });
288289
}
289290

290291
auto out_buf = std::make_unique<uint8_t[]>(outlen);
@@ -306,8 +307,9 @@ std::shared_ptr<ArrayBuffer> HybridRsaCipher::publicDecrypt(const std::shared_pt
306307
if ((err & 0xFFFFFFF) == 0x1C880004 || (err & 0xFF) == 0x04) {
307308
ERR_clear_error();
308309
EVP_PKEY_CTX_free(ctx);
309-
uint8_t* empty_buf = new uint8_t[1];
310-
return std::make_shared<NativeArrayBuffer>(empty_buf, 0, [empty_buf]() { delete[] empty_buf; });
310+
auto empty_buf = std::make_unique<uint8_t[]>(1);
311+
uint8_t* raw_ptr = empty_buf.get();
312+
return std::make_shared<NativeArrayBuffer>(empty_buf.release(), 0, [raw_ptr]() { delete[] raw_ptr; });
311313
}
312314
EVP_PKEY_CTX_free(ctx);
313315
throwOpaqueDecryptFailure();

‎packages/react-native-quick-crypto/cpp/cipher/XChaCha20Poly1305Cipher.cpp‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,42 +70,42 @@ std::shared_ptr<ArrayBuffer> XChaCha20Poly1305Cipher::final() {
7070
throw std::runtime_error("XChaCha20Poly1305Cipher: libsodium must be enabled (BLSALLOC_SODIUM)");
7171
#else
7272
if (is_cipher) {
73-
uint8_t* ciphertext = new uint8_t[data_buffer_.size()];
73+
auto ciphertext = std::make_unique<uint8_t[]>(data_buffer_.size());
7474

7575
int result =
76-
crypto_aead_xchacha20poly1305_ietf_encrypt_detached(ciphertext, auth_tag_, nullptr, data_buffer_.data(), data_buffer_.size(),
76+
crypto_aead_xchacha20poly1305_ietf_encrypt_detached(ciphertext.get(), auth_tag_, nullptr, data_buffer_.data(), data_buffer_.size(),
7777
aad_.empty() ? nullptr : aad_.data(), aad_.size(), nullptr, nonce_, key_);
7878

7979
if (result != 0) {
80-
sodium_memzero(ciphertext, data_buffer_.size());
81-
delete[] ciphertext;
80+
sodium_memzero(ciphertext.get(), data_buffer_.size());
8281
throw std::runtime_error("XChaCha20Poly1305Cipher: encryption failed");
8382
}
8483

8584
is_finalized = true;
8685
size_t ct_len = data_buffer_.size();
87-
return std::make_shared<NativeArrayBuffer>(ciphertext, ct_len, [=]() { delete[] ciphertext; });
86+
uint8_t* raw_ptr = ciphertext.get();
87+
return std::make_shared<NativeArrayBuffer>(ciphertext.release(), ct_len, [raw_ptr]() { delete[] raw_ptr; });
8888
} else {
8989
if (data_buffer_.empty()) {
9090
is_finalized = true;
9191
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
9292
}
9393

94-
uint8_t* plaintext = new uint8_t[data_buffer_.size()];
94+
auto plaintext = std::make_unique<uint8_t[]>(data_buffer_.size());
9595

9696
int result =
97-
crypto_aead_xchacha20poly1305_ietf_decrypt_detached(plaintext, nullptr, data_buffer_.data(), data_buffer_.size(), auth_tag_,
97+
crypto_aead_xchacha20poly1305_ietf_decrypt_detached(plaintext.get(), nullptr, data_buffer_.data(), data_buffer_.size(), auth_tag_,
9898
aad_.empty() ? nullptr : aad_.data(), aad_.size(), nonce_, key_);
9999

100100
if (result != 0) {
101-
sodium_memzero(plaintext, data_buffer_.size());
102-
delete[] plaintext;
101+
sodium_memzero(plaintext.get(), data_buffer_.size());
103102
throw std::runtime_error("XChaCha20Poly1305Cipher: decryption failed - authentication tag mismatch");
104103
}
105104

106105
is_finalized = true;
107106
size_t pt_len = data_buffer_.size();
108-
return std::make_shared<NativeArrayBuffer>(plaintext, pt_len, [=]() { delete[] plaintext; });
107+
uint8_t* raw_ptr = plaintext.get();
108+
return std::make_shared<NativeArrayBuffer>(plaintext.release(), pt_len, [raw_ptr]() { delete[] raw_ptr; });
109109
}
110110
#endif
111111
}
@@ -132,9 +132,10 @@ std::shared_ptr<ArrayBuffer> XChaCha20Poly1305Cipher::getAuthTag() {
132132
throw std::runtime_error("getAuthTag must be called after final()");
133133
}
134134

135-
uint8_t* tag_copy = new uint8_t[kTagSize];
136-
std::memcpy(tag_copy, auth_tag_, kTagSize);
137-
return std::make_shared<NativeArrayBuffer>(tag_copy, kTagSize, [=]() { delete[] tag_copy; });
135+
auto tag_copy = std::make_unique<uint8_t[]>(kTagSize);
136+
std::memcpy(tag_copy.get(), auth_tag_, kTagSize);
137+
uint8_t* raw_ptr = tag_copy.get();
138+
return std::make_shared<NativeArrayBuffer>(tag_copy.release(), kTagSize, [raw_ptr]() { delete[] raw_ptr; });
138139
#endif
139140
}
140141

‎packages/react-native-quick-crypto/cpp/cipher/XSalsa20Poly1305Cipher.cpp‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,38 +60,38 @@ std::shared_ptr<ArrayBuffer> XSalsa20Poly1305Cipher::final() {
6060
throw std::runtime_error("XSalsa20Poly1305Cipher: libsodium must be enabled (BLSALLOC_SODIUM)");
6161
#else
6262
if (is_cipher) {
63-
uint8_t* ciphertext = new uint8_t[data_buffer_.size()];
63+
auto ciphertext = std::make_unique<uint8_t[]>(data_buffer_.size());
6464

65-
int result = crypto_secretbox_detached(ciphertext, auth_tag_, data_buffer_.data(), data_buffer_.size(), nonce_, key_);
65+
int result = crypto_secretbox_detached(ciphertext.get(), auth_tag_, data_buffer_.data(), data_buffer_.size(), nonce_, key_);
6666

6767
if (result != 0) {
68-
sodium_memzero(ciphertext, data_buffer_.size());
69-
delete[] ciphertext;
68+
sodium_memzero(ciphertext.get(), data_buffer_.size());
7069
throw std::runtime_error("XSalsa20Poly1305Cipher: encryption failed");
7170
}
7271

7372
is_finalized = true;
7473
size_t ct_len = data_buffer_.size();
75-
return std::make_shared<NativeArrayBuffer>(ciphertext, ct_len, [=]() { delete[] ciphertext; });
74+
uint8_t* raw_ptr = ciphertext.get();
75+
return std::make_shared<NativeArrayBuffer>(ciphertext.release(), ct_len, [raw_ptr]() { delete[] raw_ptr; });
7676
} else {
7777
if (data_buffer_.empty()) {
7878
is_finalized = true;
7979
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
8080
}
8181

82-
uint8_t* plaintext = new uint8_t[data_buffer_.size()];
82+
auto plaintext = std::make_unique<uint8_t[]>(data_buffer_.size());
8383

84-
int result = crypto_secretbox_open_detached(plaintext, data_buffer_.data(), auth_tag_, data_buffer_.size(), nonce_, key_);
84+
int result = crypto_secretbox_open_detached(plaintext.get(), data_buffer_.data(), auth_tag_, data_buffer_.size(), nonce_, key_);
8585

8686
if (result != 0) {
87-
sodium_memzero(plaintext, data_buffer_.size());
88-
delete[] plaintext;
87+
sodium_memzero(plaintext.get(), data_buffer_.size());
8988
throw std::runtime_error("XSalsa20Poly1305Cipher: decryption failed - authentication tag mismatch");
9089
}
9190

9291
is_finalized = true;
9392
size_t pt_len = data_buffer_.size();
94-
return std::make_shared<NativeArrayBuffer>(plaintext, pt_len, [=]() { delete[] plaintext; });
93+
uint8_t* raw_ptr = plaintext.get();
94+
return std::make_shared<NativeArrayBuffer>(plaintext.release(), pt_len, [raw_ptr]() { delete[] raw_ptr; });
9595
}
9696
#endif
9797
}
@@ -111,9 +111,10 @@ std::shared_ptr<ArrayBuffer> XSalsa20Poly1305Cipher::getAuthTag() {
111111
throw std::runtime_error("getAuthTag must be called after final()");
112112
}
113113

114-
uint8_t* tag_copy = new uint8_t[kTagSize];
115-
std::memcpy(tag_copy, auth_tag_, kTagSize);
116-
return std::make_shared<NativeArrayBuffer>(tag_copy, kTagSize, [=]() { delete[] tag_copy; });
114+
auto tag_copy = std::make_unique<uint8_t[]>(kTagSize);
115+
std::memcpy(tag_copy.get(), auth_tag_, kTagSize);
116+
uint8_t* raw_ptr = tag_copy.get();
117+
return std::make_shared<NativeArrayBuffer>(tag_copy.release(), kTagSize, [raw_ptr]() { delete[] raw_ptr; });
117118
#endif
118119
}
119120

0 commit comments

Comments
 (0)