Skip to content

Three rare error paths in image decoders leak native resources #2586

@iurisilvio

Description

@iurisilvio

Bug

Three rare error paths in image decoding and pattern construction allocate native resources and return without releasing them. None of them are reachable in normal operation — they require a malformed image, cairo rejecting mime data, or cairo failing to create a context — but each one is a real ownership bug and each is a one- or two-line fix.

1. Image::loadGIFFromBuffer()data leaks when no color map exists

uint8_t *data = new uint8_t[naturalWidth * naturalHeight * 4];
// ...
ColorMapObject *colormap = img->ColorMap ? img->ColorMap : gif->SColorMap;

if (colormap == nullptr) {
  GIF_CLOSE_FILE(gif);
  return CAIRO_STATUS_READ_ERROR;   // <- data leaks
}

GIFs without a global color map and without a local color map on the first image descriptor are pathological but parseable up to this point. data (the full width * height * 4 decode buffer) is dropped.

2. Image::assignDataAsMime()mime_data + mime_closure leak when cairo rejects the mime

Napi::MemoryManagement::AdjustExternalMemory(env, len);

return cairo_surface_set_mime_data(_surface
  , mime_type
  , mime_data
  , len
  , clearMimeData
  , mime_closure);

mime_data is a freshly malloced copy of the JPEG buffer; mime_closure is the destruction state cairo would normally call on cleanup. If cairo_surface_set_mime_data returns non-success, cairo does not take ownership and will not call clearMimeData, so both allocations live forever. The external-memory accounting for len is also never reversed, so Napi::MemoryManagement keeps thinking the buffer is alive. Per-occurrence leak is the size of the JPEG buffer.

3. create_transparent_pattern()mask_context + mask_surface leak on cairo OOM

cairo_t *mask_context = cairo_create(mask_surface);
if (cairo_status(mask_context) != CAIRO_STATUS_SUCCESS) {
  return NULL;   // <- mask_context and mask_surface leak
}

Only reachable when cairo cannot create a context (typically OOM). Returns NULL without destroying either object.

Reproduction

These don't show measurable RSS slope in normal operation — they all sit behind error/OOM gates that aren't easy to hit deliberately from JS without poking at internals. The bugs are visible by code inspection alone, and the fixes are tiny.

Fix

  1. loadGIFFromBuffer: delete[] data before returning the no-colormap error.
  2. assignDataAsMime: capture the status of cairo_surface_set_mime_data; on non-success, reverse the AdjustExternalMemory(env, len) and free both mime_data and mime_closure before returning.
  3. create_transparent_pattern: destroy mask_context and mask_surface before returning NULL.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions