Possible use-after-free of a JS Buffer captured by the async `Download` worker

[OvOhao]

Possible use-after-free of a JS Buffer captured by the async Download worker

I found a possible borrowed-buffer-across-async use-after-free in S7Client::Download.
When a completion callback is supplied, Download passes the raw backing-store pointer of the
JS Buffer block argument via node::Buffer::Data(info[1]) (together with its length) to a
libuv-threadpool IOWorker (caller DOWNLOAD). The worker keeps only a bare void* pData; no
napi_reference/Nan::Persistent is created on the buffer, and the JS wrapper does not retain
it. Once the synchronous call returns undefined, nothing keeps the Buffer alive, so a GC can
free or relocate its backing store while the worker thread is still reading from that pointer
inside snap7Client->Download(...) → use-after-free.

File: src/node_snap7_client.cpp

Function: S7Client::Download

} else {
    Nan::Callback *callback = new Nan::Callback(info[2].As<v8::Function>());
    Nan::AsyncQueueWorker(new IOWorker(callback, s7client, DOWNLOAD
      , node::Buffer::Data(info[1].As<v8::Object>()), Nan::To<int32_t>(info[0]).FromJust()
      , static_cast<int>(node::Buffer::Length(info[1].As<v8::Object>()))));
    info.GetReturnValue().SetUndefined();
}

The worker dereferences the borrowed pointer on the threadpool thread (IOWorker::Execute()):

case DOWNLOAD:
    returnValue = s7client->snap7Client->Download(int1, pData, int2);

Path:

1. node::Buffer::Data(info[1]) returns a pointer into the block Buffer's V8 backing store.
2. That raw pointer is stored in the worker's pData and the worker is queued; the buffer is
   never pinned with a persistent reference.
3. The sync call returns immediately.
4. If JS drops the buffer, a GC before/while Execute() runs frees or moves the backing store;
   the worker thread then reads freed memory when uploading the block to the PLC → UAF.

JS trigger (if applicable):

const snap7 = require('node-snap7');
const c = new snap7.S7Client();
c.Download(-1, Buffer.alloc(1024), (err) => {});   // buffer not retained
for (let i = 0; i < 1e6; i++) ({});                // provoke GC while the worker runs

Suggested fix: pin the block buffer for the worker's lifetime (store an Nan::Persistent
reference — e.g. SaveToPersistent on the IOWorker), or copy its bytes into worker-owned
storage before queuing, releasing/freeing in HandleOKCallback.

[mathiask88]

Thanks for reporting, yes you are right. A copy would be the safest here. I am working in my little spare time on a NAPI rewrite. If I remember right, I fixed it there already.

[OvOhao]

If you could credit me as a reporter for my contributions to security advisory I will be thankful.