Skip to content

Commit a1aed38

Browse files
committed
src: verify that released DataPointers aren't on secure heap
The ByteSource does not currently know how to free a DataPointer allocated on the secure heap, so just verify. DataPointers on the secure heap are not something that users can allocate on their own. Their use is rare. Eventually ByteSource is going to be refactored around ncrypto APIs so these additional checks should be temporary.
1 parent e14ec69 commit a1aed38

File tree

11 files changed

+24
-2
lines changed

11 files changed

+24
-2
lines changed

deps/ncrypto/ncrypto.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,8 @@ class DataPointer final {
473473
};
474474
}
475475

476+
bool isSecure() const { return secure_; }
477+
476478
private:
477479
void* data_ = nullptr;
478480
size_t len_ = 0;

src/crypto/crypto_dh.cc

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,17 @@ void DiffieHellman::MemoryInfo(MemoryTracker* tracker) const {
5555

5656
namespace {
5757
MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
58+
struct Flag {
59+
bool secure;
60+
};
5861
auto backing = ArrayBuffer::NewBackingStore(
5962
data.get(),
6063
data.size(),
61-
[](void* data, size_t len, void* ptr) { DataPointer free_me(data, len); },
62-
nullptr);
64+
[](void* data, size_t len, void* ptr) {
65+
std::unique_ptr<Flag> flag(static_cast<Flag*>(ptr));
66+
DataPointer free_me(data, len, flag->secure);
67+
},
68+
new Flag { data.isSecure() });
6369
data.release();
6470

6571
auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
@@ -482,6 +488,7 @@ ByteSource StatelessDiffieHellmanThreadsafe(const EVPKeyPointer& our_key,
482488
const EVPKeyPointer& their_key) {
483489
auto dp = DHPointer::stateless(our_key, their_key);
484490
if (!dp) return {};
491+
DCHECK(!dp.isSecure());
485492

486493
return ByteSource::Allocated(dp.release());
487494
}

src/crypto/crypto_ec.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ bool ECDHBitsTraits::DeriveBits(Environment* env,
449449

450450
auto data = ctx.derive();
451451
if (!data) return false;
452+
DCHECK(!data.isSecure());
452453

453454
*out = ByteSource::Allocated(data.release());
454455
break;
@@ -572,12 +573,14 @@ WebCryptoKeyExportStatus EC_Raw_Export(const KeyObjectData& key_data,
572573
case kKeyTypePrivate: {
573574
auto data = m_pkey.rawPrivateKey();
574575
if (!data) return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
576+
DCHECK(!data.isSecure());
575577
*out = ByteSource::Allocated(data.release());
576578
break;
577579
}
578580
case kKeyTypePublic: {
579581
auto data = m_pkey.rawPublicKey();
580582
if (!data) return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
583+
DCHECK(!data.isSecure());
581584
*out = ByteSource::Allocated(data.release());
582585
break;
583586
}

src/crypto/crypto_hash.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) {
405405
if (!data) [[unlikely]] {
406406
return ThrowCryptoError(env, ERR_get_error());
407407
}
408+
DCHECK(!data.isSecure());
408409

409410
hash->digest_ = ByteSource::Allocated(data.release());
410411
}
@@ -504,6 +505,7 @@ bool HashTraits::DeriveBits(
504505
if (!data) [[unlikely]]
505506
return false;
506507

508+
DCHECK(!data.isSecure());
507509
*out = ByteSource::Allocated(data.release());
508510
}
509511

src/crypto/crypto_hkdf.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ bool HKDFTraits::DeriveBits(
117117
params.length);
118118
if (!dp) return false;
119119

120+
DCHECK(!data.isSecure());
120121
*out = ByteSource::Allocated(dp.release());
121122
return true;
122123
}

src/crypto/crypto_hmac.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ bool HmacTraits::DeriveBits(
258258
if (!buf) [[unlikely]]
259259
return false;
260260

261+
DCHECK(!buf.isSecure());
261262
*out = ByteSource::Allocated(buf.release());
262263

263264
return true;

src/crypto/crypto_pbkdf2.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ bool PBKDF2Traits::DeriveBits(Environment* env,
126126
params.length);
127127

128128
if (!dp) return false;
129+
DCHECK(!dp.isSecure());
129130
*out = ByteSource::Allocated(dp.release());
130131
return true;
131132
}

src/crypto/crypto_rsa.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ WebCryptoCipherStatus RSA_Cipher(Environment* env,
204204

205205
auto data = cipher(m_pkey, nparams, in);
206206
if (!data) return WebCryptoCipherStatus::FAILED;
207+
DCHECK(!data.isSecure());
207208

208209
*out = ByteSource::Allocated(data.release());
209210
return WebCryptoCipherStatus::OK;

src/crypto/crypto_scrypt.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ bool ScryptTraits::DeriveBits(
140140
params.length);
141141

142142
if (!dp) return false;
143+
DCHECK(!dp.isSecure());
143144
*out = ByteSource::Allocated(dp.release());
144145
return true;
145146
}

src/crypto/crypto_sig.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,13 +682,15 @@ bool SignTraits::DeriveBits(
682682
crypto::CheckThrow(env, SignBase::Error::PrivateKey);
683683
return false;
684684
}
685+
DCHECK(!data.isSecure());
685686
*out = ByteSource::Allocated(data.release());
686687
} else {
687688
auto data = context.sign(params.data);
688689
if (!data) [[unlikely]] {
689690
crypto::CheckThrow(env, SignBase::Error::PrivateKey);
690691
return false;
691692
}
693+
DCHECK(!data.isSecure());
692694
auto bs = ByteSource::Allocated(data.release());
693695

694696
if (UseP1363Encoding(key, params.dsa_encoding)) {

0 commit comments

Comments
 (0)