Commit fdb96fe ("Address SF bug #165 EGifSpew leaks GifFileOut->SColorMap") introduced a pointer-reuse branch in EGifPutScreenDesc:
} else if (ColorMap == old_map) {
/* Reuse existing map to avoid a redundant copy. */
GifFile->SColorMap = old_map;
}
Previous behavior (5.2.2): EGifPutScreenDesc always copied the ColorMap argument via GifMakeMapObject, regardless of whether it matched the existing GifFile->SColorMap. The caller retained ownership of the original; giflib owned the copy and freed it in EGifCloseFile.
New behavior (6.1.2+, still present in 6.1.3): When ColorMap == GifFile->SColorMap (same pointer), EGifPutScreenDesc reuses the pointer instead of copying. giflib now considers itself the owner and will free it in EGifCloseFile. If the caller also frees the original (which was safe in 5.2.2), this results in a double-free.
Minimal repro
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <gif_lib.h>
static int write_func(GifFileType *gif, const GifByteType *buf, int len) {
(void)gif; (void)buf;
return len;
}
int main(void) {
int error = 0;
GifFileType *gif = EGifOpen(NULL, write_func, &error);
if (!gif) { fprintf(stderr, "EGifOpen failed: %d\n", error); return 1; }
ColorMapObject *caller_map = GifMakeMapObject(256, NULL);
if (!caller_map) { fprintf(stderr, "GifMakeMapObject failed\n"); EGifCloseFile(gif, &error); return 1; }
for (int i = 0; i < 256; i++) {
caller_map->Colors[i].Red = caller_map->Colors[i].Green = caller_map->Colors[i].Blue = i;
}
/* Set SColorMap then pass the same pointer to EGifPutScreenDesc */
gif->SColorMap = caller_map;
EGifPutScreenDesc(gif, 1, 1, 8, 0, caller_map);
/* Write minimal 1x1 image */
EGifPutImageDesc(gif, 0, 0, 1, 1, false, NULL);
GifPixelType pixel = 0;
EGifPutLine(gif, &pixel, 1);
EGifCloseFile(gif, &error); /* frees gif->SColorMap (== caller_map in 6.1.2+) */
GifFreeMapObject(caller_map); /* DOUBLE FREE on 6.1.2+ */
printf("Done\n");
return 0;
}
Build with: gcc -fsanitize=address -g -o repro repro.c -lgif
Impact: Any caller that (1) sets GifFile->SColorMap directly, (2) passes the same pointer to EGifPutScreenDesc, and (3) manages its own cleanup of the ColorMapObject will hit a double-free after upgrading from 5.2.2 to 6.1.2+.
Suggested fix: The leak in EGifSpew could be fixed inside EGifSpew itself (free the old map before calling EGifPutScreenDesc) rather than changing the ownership contract of EGifPutScreenDesc for all callers.