Skip to content

Possible per-call leak of the compressed output buffer in PQCompress::Invoke #35

Description

@OvOhao

Possible per-call leak of the compressed output buffer in PQCompress::Invoke

I found a possible per-operation memory leak in PQCompress::Invoke (the .compress() method). pngquant(in, out, ...) produces the quantized PNG by malloc/realloc-ing out->png_data inside the vendored library's write callback. Invoke then copies those bytes into a JS buffer with napi_create_buffer_copy and afterwards frees the cmd_argv slots and the in/out struct wrappers — but it never frees out->png_data. The compressed output buffer is therefore leaked on every successful compress call.

File: src/PQCompress.cc

Function: PQCompress::Invoke

  rwpngTyp in  = (rwpngTyp) malloc(sizeof(struct rwpng_data));
  rwpngTyp out = (rwpngTyp) malloc(sizeof(struct rwpng_data));
  ...
  memset(out, '\0', sizeof(struct rwpng_data));

  int ret = pngquant(in, out, cmd_argc, cmd_argv);   // allocates out->png_data on success
  ...
  status = napi_create_buffer_copy(env, (size_t) out->length, (char *) out->png_data, nullptr, &result);
  assert(status == napi_ok);

  for (i = 0; i < 32; i++) { free(cmd_argv[i]); }
  free(in);
  free(out);                                          // frees the struct, NOT out->png_data

  return result;
  1. out is zeroed, so out->png_data starts NULL.
  2. Inside pngquant(...), the write path (rwpng.c) allocates the output: image->png_data = (png_bytep) malloc(len) / realloc(image->png_data, len). On success out->png_data owns a heap buffer holding the quantized PNG.
  3. napi_create_buffer_copy makes an independent copy into the returned JS buffer, so the native out->png_data allocation is no longer referenced after this line.
  4. The cleanup frees cmd_argv[i], in, and out, but there is no free(out->png_data) — the malloc'd compressed image is leaked. free(out) only releases the 3-word struct, not the buffer it points to.
  5. This runs on every .compress() call, so throughput-oriented use leaks one compressed-PNG-sized buffer per image.

(On the ret != 0 error branch the code sets out->png_data = in->png_data, i.e. the input pointer, so freeing must be limited to the success branch where out->png_data is the library-allocated output.)

This is the addon's own glue code (src/PQCompress.cc), synchronous, with no destructor or RAII owner for out->png_data.

JS trigger (if applicable):

const { Pngquant } = require('pngquant-native');
const png = require('fs').readFileSync('in.png');
for (let i = 0; i < 100000; i++) new Pngquant(png, '').compress(); // RSS grows per call

Suggested fix: after napi_create_buffer_copy succeeds (success branch only), free(out->png_data) before free(out); guard against the error branch where out->png_data aliases in->png_data.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions