0

I'm working on a Node addon that encrypts data using Windows DPAPI. I'm passing two Javascript Uint8Array to the C++ code using NAN.

This is what the typescript interface looks like:

export interface DpapiBindings{
    protectData(dataToEncrypt: Uint8Array, optionalEntropy: Uint8Array, scope: string): Uint8Array
}

I'd like to then create a Node::Buffer in the C++ code:

void ProtectData( Nan::NAN_METHOD_ARGS_TYPE info)
{
    v8::Isolate* isolate = info.GetIsolate();

   // 
    auto buffer = node::Buffer::Data(info[0]);
    auto len = node::Buffer::Length(info[0]);

    DATA_BLOB entropyBlob;
    entropyBlob.pbData = nullptr;
    if (!info[1]->IsNull())
    {
        entropyBlob.pbData = reinterpret_cast<BYTE*>(node::Buffer::Data(info[1]));
        entropyBlob.cbData = node::Buffer::Length(info[1]);
    }

    DATA_BLOB dataIn;
    DATA_BLOB dataOut;

    // initialize input data
    dataIn.pbData = reinterpret_cast<BYTE*>(buffer);
    dataIn.cbData = len;

    success = CryptProtectData(
            &dataIn,
            nullptr,
            entropyBlob.pbData ? &entropyBlob : nullptr,
            nullptr, 
            nullptr,
            flags,
            &dataOut);

    auto returnBuffer = Nan::CopyBuffer(reinterpret_cast<const char*>(dataOut.pbData), dataOut.cbData).ToLocalChecked();
    LocalFree(dataOut.pbData);

    info.GetReturnValue().Set(returnBuffer);

}

I'm new to C++, my question is: When working with node::Buffer::Data and node::Buffer::Length in C++ code, and calling into CryptProtectData, do I need to worry about buffer overflows? If so, how do I protect against it? Should I be appending a null char to buffer and len?

sgonzalez
  • 741
  • 6
  • 20
  • See [this](https://stackoverflow.com/a/62676223/980129). – Manuel Jul 09 '20 at 19:01
  • @Manuel, thanks that's helpful. I've edited my question with a follow up. – sgonzalez Jul 09 '20 at 19:38
  • 1
    @gonzalez you set the length in `dataIn.cbData = len` so no, no overflow. As for the `null` `char`, if it is there at input it will be part of the encrypted buffer. You can check for it in `buffer[len - 1]`. If it isn't there you'll need to add it after unencryption (if it is a string.) – Manuel Jul 09 '20 at 19:49
  • @Manuel To make sure I understand correctly: overflow for dataIn.pbData is not a problem because we set the length in dataIn.cbData, so we know how many bytes there are. Is this the same case for dataIn.cbData? Do I need to ensure that dataIn.cbData (in my case node::Buffer::Length(info[1])) is null terminated since we are not passing in the length? – sgonzalez Jul 09 '20 at 20:15
  • 1
    dataIn.cbData has the size of the buffer received, whether this has the `null` or not I can't say as it is a `Uint8Array` from JS and it is a byte array. I guess you don't need it. – Manuel Jul 09 '20 at 20:42

1 Answers1

1

No, you don't need to worry about overflow. The dataIn and dataOut structures are pointers with a length.

BufferCopy is not what you want to use though. You want to use: Nan::MaybeLocal<v8::Object> Nan::NewBuffer(char* data, uint32_t size).

https://github.com/nodejs/nan/blob/master/doc/buffers.md#api_nan_new_buffer

and make sure you free the dataOut.pbData memory when you're done (i see you are with the LocalFree call.) the reason it can't overflow is that CryptProtectData allocates that buffer based on the size it needs.

bmacnaughton
  • 4,950
  • 3
  • 27
  • 36