Leaking Bytes in the Fast Lane: ImageMagick OpenCL DoS

ImageMagick is the heavy lifter of the internet's image processing infrastructure. From PHP extensions to command-line cron jobs, it is everywhere. To keep up with the demand for speed, ImageMagick leverages OpenCL to offload processing to GPUs and accelerators. It's a noble goal: crunch pixels faster.

But hardware acceleration requires configuration. To track performance, ImageMagick benchmarks available OpenCL devices and caches the results in an XML file, typically residing in ~/.cache/ImageMagick/ImagemagickOpenCLDeviceProfile.xml. This file acts as a persistent memory of the hardware's capabilities.

Here is the catch: Parsing XML in C is notoriously painful. It requires manual memory management for every tag, attribute, and text node. And in the LoadOpenCLDeviceBenchmark function, the developers made a classic mistake: they focused on the container and forgot about the contents.

The vulnerability lies within MagickCore/opencl.c. When ImageMagick initializes, it reads the benchmark XML to avoid re-running expensive tests. The function responsible, LoadOpenCLDeviceBenchmark, iterates through the file looking for <device> tags.

When it finds a device, it allocates a MagickCLDeviceBenchmark structure. As it parses attributes like platform, vendor, name, and version, it allocates dynamic memory for each string using ConstantString(). These pointers are stored inside the struct.

Here is the logic flaw: The code assumes a happy path where the XML tag is closed properly (e.g., />). If the tag is malformed—specifically, if it is never closed—the parser hits an error condition or simply reaches the end of the loop. In this exit path, the code creates a 'zombie' struct. It frees the device_benchmark struct itself, but it fails to free the strings pointed to by the struct members before destroying it. The container is gone, but the data it held is left floating in the heap, causing a memory leak.

Let's look at the vulnerable logic in MagickCore/opencl.c. The parser allocates memory for the attributes as soon as it sees them:

/* Inside the parsing loop */
if (LocaleCompare(keyword,"name") == 0)
  {
    /* Allocates memory for 'name' */
    device_benchmark->name=ConstantString(token);
    continue;
  }
// ... similar logic for platform, vendor, etc.

When the loop terminates (potentially prematurely due to malformed XML), the cleanup code looks like this:

/* The Vulnerable Cleanup */
token=(char *) RelinquishMagickMemory(token);
/* The struct is freed... */
device_benchmark=(MagickCLDeviceBenchmark *) RelinquishMagickMemory(device_benchmark);
/* ...but device_benchmark->name is NOT freed! */

This is the programming equivalent of demolishing a house but leaving all the furniture hovering in mid-air. The pointer to the name string is inside device_benchmark. Once you free device_benchmark, you lose the address of name, making it impossible to free later.

To exploit this, we don't need complex heap feng shui or ROP chains. We just need to feed ImageMagick a bad configuration file. The target is the OpenCL cache file.

An attacker can craft a file named ImagemagickOpenCLDeviceProfile.xml with a device tag that opens but never closes:

<opencl-device-profile>
  <!-- Open the tag, allocate the strings, but never close it with /> -->
  <device platform="LeakMe" vendor="LeakMeVendor" name="LeakMeName" version="1.0"
</opencl-device-profile>

When ImageMagick runs (even for a simple operation like converting a blank image), it parses this file. It allocates memory for "LeakMe", "LeakMeVendor", etc. It reaches the end of the file looking for the closing bracket />, doesn't find it, and hits the cleanup routine. The strings are leaked.

Using AddressSanitizer (ASAN), we can see the exact carnage:

==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #1 ... in LoadOpenCLDeviceBenchmark MagickCore/opencl.c:878

While ~200 bytes per run sounds trivial, this accumulates rapidly in environments where ImageMagick is spawned repeatedly (e.g., a web worker processing user uploads) or in long-running daemon processes that reload configurations.

The remediation is straightforward: you must free the members before you free the struct. The maintainers patched this by explicitly relinquishing memory for every string attribute before destroying the parent object.

Here is the corrected logic:

/* The Corrected Cleanup */
if (device_benchmark != (MagickCLDeviceBenchmark *) NULL)
{
  /* Free the members first */
  if (device_benchmark->platform_name != (char *) NULL)
    device_benchmark->platform_name=RelinquishMagickMemory(device_benchmark->platform_name);
  if (device_benchmark->vendor_name != (char *) NULL)
    device_benchmark->vendor_name=RelinquishMagickMemory(device_benchmark->vendor_name);
  if (device_benchmark->name != (char *) NULL)
    device_benchmark->name=RelinquishMagickMemory(device_benchmark->name);
  if (device_benchmark->version != (char *) NULL)
    device_benchmark->version=RelinquishMagickMemory(device_benchmark->version);
 
  /* Then free the struct */
  device_benchmark=(MagickCLDeviceBenchmark *) RelinquishMagickMemory(device_benchmark);
}

This ensures that regardless of how the parsing ends (success or malformed error), the heap is returned to a clean state.