Automattic / Automattic/node-canvas

Possible use-after-free in PDF stream chunks

Open
#2,603 0 comments 0 reactions 0 assignees View on GitHub
Security
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.