Skip to content

Commit 4755ad5

Browse files
kvakilruyadorno
authored andcommitted
src: remove usages of GetBackingStore in crypto
This removes all usages of GetBackingStore in `crypto`. See the linked issue for an explanation. Note: I am not sure of the lifetime semantics intended by `ArrayBufferOrViewContents` -- I am pretty sure it is correct based on a manual audit of the callsites, but please ensure that it is correct. Refs: #32226 Refs: #43921 PR-URL: #44079 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 2f904bc commit 4755ad5

File tree

3 files changed

+29
-16
lines changed

3 files changed

+29
-16
lines changed

src/crypto/README.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,15 @@ the `ByteSource::Builder` without releasing it as a `ByteSource`.
112112

113113
### `ArrayBufferOrViewContents`
114114

115-
The `ArrayBufferOfViewContents` class is a helper utility that abstracts
115+
The `ArrayBufferOrViewContents` class is a helper utility that abstracts
116116
`ArrayBuffer`, `TypedArray`, or `DataView` inputs and provides access to
117117
their underlying data pointers. It is used extensively through `src/crypto`
118118
to make it easier to deal with inputs that allow any `ArrayBuffer`-backed
119119
object.
120120

121+
The lifetime of `ArrayBufferOrViewContents` should not exceed the
122+
lifetime of its input.
123+
121124
### Key objects
122125

123126
Most crypto operations involve the use of keys -- cryptographic inputs

src/crypto/crypto_cipher.cc

+6-9
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,8 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
536536
if (UNLIKELY(key_buf.size() > INT_MAX))
537537
return THROW_ERR_OUT_OF_RANGE(env, "key is too big");
538538

539-
ArrayBufferOrViewContents<unsigned char> iv_buf;
540-
if (!args[2]->IsNull())
541-
iv_buf = ArrayBufferOrViewContents<unsigned char>(args[2]);
539+
ArrayBufferOrViewContents<unsigned char> iv_buf(
540+
!args[2]->IsNull() ? args[2] : Local<Value>());
542541

543542
if (UNLIKELY(!iv_buf.CheckSizeInt32()))
544543
return THROW_ERR_OUT_OF_RANGE(env, "iv is too big");
@@ -1061,12 +1060,10 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
10611060
return THROW_ERR_OSSL_EVP_INVALID_DIGEST(env);
10621061
}
10631062

1064-
ArrayBufferOrViewContents<unsigned char> oaep_label;
1065-
if (!args[offset + 3]->IsUndefined()) {
1066-
oaep_label = ArrayBufferOrViewContents<unsigned char>(args[offset + 3]);
1067-
if (UNLIKELY(!oaep_label.CheckSizeInt32()))
1068-
return THROW_ERR_OUT_OF_RANGE(env, "oaep_label is too big");
1069-
}
1063+
ArrayBufferOrViewContents<unsigned char> oaep_label(
1064+
!args[offset + 3]->IsUndefined() ? args[offset + 3] : Local<Value>());
1065+
if (UNLIKELY(!oaep_label.CheckSizeInt32()))
1066+
return THROW_ERR_OUT_OF_RANGE(env, "oaep_label is too big");
10701067

10711068
std::unique_ptr<BackingStore> out;
10721069
if (!Cipher<operation, EVP_PKEY_cipher_init, EVP_PKEY_cipher>(

src/crypto/crypto_util.h

+19-6
Original file line numberDiff line numberDiff line change
@@ -699,24 +699,30 @@ template <typename T>
699699
class ArrayBufferOrViewContents {
700700
public:
701701
ArrayBufferOrViewContents() = default;
702+
ArrayBufferOrViewContents(const ArrayBufferOrViewContents&) = delete;
703+
void operator=(const ArrayBufferOrViewContents&) = delete;
702704

703705
inline explicit ArrayBufferOrViewContents(v8::Local<v8::Value> buf) {
706+
if (buf.IsEmpty()) {
707+
return;
708+
}
709+
704710
CHECK(IsAnyByteSource(buf));
705711
if (buf->IsArrayBufferView()) {
706712
auto view = buf.As<v8::ArrayBufferView>();
707713
offset_ = view->ByteOffset();
708714
length_ = view->ByteLength();
709-
store_ = view->Buffer()->GetBackingStore();
715+
data_ = view->Buffer()->Data();
710716
} else if (buf->IsArrayBuffer()) {
711717
auto ab = buf.As<v8::ArrayBuffer>();
712718
offset_ = 0;
713719
length_ = ab->ByteLength();
714-
store_ = ab->GetBackingStore();
720+
data_ = ab->Data();
715721
} else {
716722
auto sab = buf.As<v8::SharedArrayBuffer>();
717723
offset_ = 0;
718724
length_ = sab->ByteLength();
719-
store_ = sab->GetBackingStore();
725+
data_ = sab->Data();
720726
}
721727
}
722728

@@ -726,7 +732,7 @@ class ArrayBufferOrViewContents {
726732
// length is zero, so we have to return something.
727733
if (size() == 0)
728734
return &buf;
729-
return reinterpret_cast<T*>(store_->Data()) + offset_;
735+
return reinterpret_cast<T*>(data_) + offset_;
730736
}
731737

732738
inline T* data() {
@@ -735,7 +741,7 @@ class ArrayBufferOrViewContents {
735741
// length is zero, so we have to return something.
736742
if (size() == 0)
737743
return &buf;
738-
return reinterpret_cast<T*>(store_->Data()) + offset_;
744+
return reinterpret_cast<T*>(data_) + offset_;
739745
}
740746

741747
inline size_t size() const { return length_; }
@@ -775,7 +781,14 @@ class ArrayBufferOrViewContents {
775781
T buf = 0;
776782
size_t offset_ = 0;
777783
size_t length_ = 0;
778-
std::shared_ptr<v8::BackingStore> store_;
784+
void* data_ = nullptr;
785+
786+
// Declaring operator new and delete as deleted is not spec compliant.
787+
// Therefore declare them private instead to disable dynamic alloc
788+
void* operator new(size_t);
789+
void* operator new[](size_t);
790+
void operator delete(void*);
791+
void operator delete[](void*);
779792
};
780793

781794
v8::MaybeLocal<v8::Value> EncodeBignum(

0 commit comments

Comments
 (0)