Automattic / Automattic/node-canvas
Possible use-after-free in PDF stream chunks
- Dominant language
- JavaScript
- Stars
- 10.7k
- Forks
- 1.2k
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 1
Description
# Possible use-after-free in PDF stream chunks
I found a possible use-after-free in PDF stream chunks returned by `streamPDFSync()`.
Files: `src/closure.h`, `src/Canvas.cc`, `lib/pdfstream.js`
Functions: `Closure::writeVec`, `Canvas::StreamPDFSync`, `streamPDF`, `Canvas::destroySurface`
Relevant code:
```cpp
struct Closure {
std::vector vec;
static cairo_status_t writeVec(void *c, const uint8_t *odata, unsigned len) {
Closure* closure = static_cast(c);
closure->vec.insert(closure->vec.end(), odata, odata + len);
return CAIRO_STATUS_SUCCESS;
}
};
```
`StreamPDFSync()` streams pointers into that vector:
```cpp
PdfSvgClosure *closure = static_cast(_closure);
PdfStreamInfo streaminfo;
streaminfo.fn = fn;
streaminfo.data = &closure->vec[0];
streaminfo.len = closure->vec.size();
cairo_status_t status = canvas_write_to_pdf_stream(ensureSurface(), streamPDF, &streaminfo);
```
The stream callback creates an external `Buffer` over the supplied pointer:
```cpp
Napi::Value buf = Napi::Buffer::New(env, (uint8_t *)(data), len);
streaminfo->fn.MakeCallback(env.Global(), { env.Null(), buf, Napi::Number::New(env, len) }, async);
```
The source comment already points out the lifetime issue:
```cpp
// TODO this is technically wrong, we're returning a pointer to the data in a
// vector in a class with automatic storage duration. If the canvas goes out
// of scope while we're in the handler, a use-after-free could happen.
```
The closure storage is freed when the canvas surface is destroyed:
```cpp
if (_closure) {
delete _closure;
_closure = nullptr;
}
```
JavaScript receives the chunks through the public stream API:
```js
this.canvas.streamPDFSync((err, chunk, len) => {
if (len) {
this.push(chunk)
} else {
this.push(null)
}
}, this.options)
```
If user code retains a chunk after the canvas has been collected or destroyed,
the `Buffer` can still point into the freed `Closure::vec` storage.
Suggested fix: make each PDF chunk own its bytes, for example by using
`Napi::Buffer::Copy(env, data, len)`, or attach an owner/finalizer that
keeps the backing storage alive until the JS `Buffer` is finalized.
Contributor guide
No contributing guide indexed for this repository
Research direction
Trace the lifetime of Closure::vec through src/closure.h, src/Canvas.cc, and lib/pdfstream.js, starting with Canvas::StreamPDFSync and the streamPDF callback. Exercise the public streamPDFSync API while retaining a chunk beyond canvas destruction, then verify the chunk remains valid and that the ownership behavior is covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, javascript
- Domain
- backend-api-design, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100