Possible use-after-free / invalid-free: node::Buffer::New aliases freed std::string storage in EncodePagesResult
I found a possible memory-safety defect in EncodePagesResult. Each rendered page is returned to JS by constructing a Node Buffer with node::Buffer::New(isolate, (char*)it->c_str(), it->size()), passing a pointer into a std::string stored in req->result. The 3-argument node::Buffer::New(Isolate*, char*, size_t) overload does not copy — it adopts the pointer as the Buffer's backing store (and, on modern Node, frees it with free() when the Buffer is garbage-collected). Because that memory is actually owned by the std::string (and is not malloc'd), two things go wrong: (a) the JS Buffer aliases string storage that is freed while JS may still hold the Buffer, and (b) the backing pointer is later free()d twice — once by the adopting Buffer and once by the std::string destructor.
File: src/node_pdfium.cc
Function: EncodePagesResult (invoked from RenderAsyncAfter and the sync branch of render)
void EncodePagesResult(const std::vector<std::string>& iResult, v8::Handle<v8::Array> oResult) {
MY_NODE_MODULE_ISOLATE_DECL;
int i = 0;
for (std::vector<std::string>::const_iterator it = iResult.begin(); it != iResult.end(); ++it) {
v8::MaybeLocal<v8::Object> objTemp =
node::Buffer::New(MY_NODE_MODULE_ISOLATE_PRE (char*)it->c_str(), it->size()); // adopts c_str()
v8::Local<v8::Value> data(objTemp.ToLocalChecked());
oResult->Set(i++, data);
}
}
void RenderAsyncAfter(uv_work_t *r) {
...
v8::Local<v8::Array> result = V8_VALUE_NEW_DEFAULT_V_0_11_10(Array);
EncodePagesResult(req->result, result); // Buffers alias req->result[i] storage
...
callback->Call(MY_NODE_MODULE_CONTEXT, 2, argv);
req->callback.Reset();
delete req; // destroys the std::strings -> frees the aliased storage
...
}
req->result is a std::vector<std::string>; each string holds one rendered page (PNG/PPM bytes).
EncodePagesResult hands JS a Buffer whose backing store is it->c_str() — the string's internal buffer — via the ownership-taking Buffer::New overload, with no copy.
- In the async path, right after the JS callback returns,
delete req runs the std::string destructors, freeing that storage. Any JS code that retained a page Buffer (the normal use — the whole point of the call is to keep the rendered pages) now reads freed memory: a use-after-free with attacker-influenced content disclosure.
- In addition, on modern Node the adopted pointer is
free()d when the Buffer is GC'd; since it was allocated by the std::string allocator (not malloc) and is also freed by the string destructor, this is an invalid free / double free (CWE-762 / CWE-415).
- The same aliasing happens in the synchronous branch of
render (EncodePagesResult(req->result, result) at the end of render), where the MemValueBase<RenderAsyncReq> and its strings are destroyed when render returns, leaving the returned Buffers dangling.
src/node_pdfium.cc is the addon's own code (the PDFium library lives under third_party/), and there is no copy or lifetime extension of the string data.
JS trigger (if applicable):
const pdfium = require('node-pdfium');
pdfium.render({ data: fs.readFileSync('a.pdf'), outputFormat: 'PNG' }, (err, pages) => {
const keep = pages[0]; // retains a Buffer aliasing freed std::string storage
setImmediate(() => keep[0]); // reads/uses freed memory (UAF); free()-on-GC is an invalid free
});
Suggested fix: copy the bytes instead of adopting them — use node::Buffer::Copy(isolate, it->c_str(), it->size()) (which allocates and copies), so the returned Buffer owns independent, correctly-malloc'd memory and the std::strings can be destroyed safely.
Possible use-after-free / invalid-free:
node::Buffer::Newaliases freedstd::stringstorage inEncodePagesResultI found a possible memory-safety defect in
EncodePagesResult. Each rendered page is returned to JS by constructing a Node Buffer withnode::Buffer::New(isolate, (char*)it->c_str(), it->size()), passing a pointer into astd::stringstored inreq->result. The 3-argumentnode::Buffer::New(Isolate*, char*, size_t)overload does not copy — it adopts the pointer as the Buffer's backing store (and, on modern Node, frees it withfree()when the Buffer is garbage-collected). Because that memory is actually owned by thestd::string(and is notmalloc'd), two things go wrong: (a) the JS Buffer aliases string storage that is freed while JS may still hold the Buffer, and (b) the backing pointer is laterfree()d twice — once by the adopting Buffer and once by thestd::stringdestructor.File:
src/node_pdfium.ccFunction:
EncodePagesResult(invoked fromRenderAsyncAfterand the sync branch ofrender)req->resultis astd::vector<std::string>; each string holds one rendered page (PNG/PPM bytes).EncodePagesResulthands JS a Buffer whose backing store isit->c_str()— the string's internal buffer — via the ownership-takingBuffer::Newoverload, with no copy.delete reqruns thestd::stringdestructors, freeing that storage. Any JS code that retained a page Buffer (the normal use — the whole point of the call is to keep the rendered pages) now reads freed memory: a use-after-free with attacker-influenced content disclosure.free()d when the Buffer is GC'd; since it was allocated by thestd::stringallocator (notmalloc) and is also freed by the string destructor, this is an invalid free / double free (CWE-762 / CWE-415).render(EncodePagesResult(req->result, result)at the end ofrender), where theMemValueBase<RenderAsyncReq>and its strings are destroyed whenrenderreturns, leaving the returned Buffers dangling.src/node_pdfium.ccis the addon's own code (the PDFium library lives underthird_party/), and there is no copy or lifetime extension of the string data.JS trigger (if applicable):
Suggested fix: copy the bytes instead of adopting them — use
node::Buffer::Copy(isolate, it->c_str(), it->size())(which allocates and copies), so the returned Buffer owns independent, correctly-malloc'd memory and thestd::strings can be destroyed safely.