double free or corruption error in "vips_image_new_from_buffer"

[dineshkannaa]

I have loaded a heic image , using
vips_image_new_from_buffer(buffer, fileLen, "", "access",VIPS_ACCESS_SEQUENTIAL,NULL)
and I got this after .

note: I didnt link libheif, just tried this to check error handling .

double free or corruption (!prev): 0x00007fafa004a220 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7c619)[0x7fb3bb4ca619]
/lib64/libc.so.6(+0x7e918)[0x7fb3bb4cc918]
/lib64/libc.so.6(realloc+0x1b2)[0x7fb3bb4ce752]
/lib64/libglib-2.0.so.0(g_realloc+0x16)[0x7faf269bab66]
/lib64/libgobject-2.0.so.0(+0xa852)[0x7fafb90d3852]
/lib64/libgobject-2.0.so.0(g_value_register_transform_func+0x200)[0x7fafb90fe5b0]
/vips_8_7_0/lib/libvips.so.42(vips_area_get_type+0x49)[0x7fafb945d509]
/vips_8_7_0/lib/libvips.so.42(vips__meta_init_types+0x13)[0x7fafb945eb33]
/vips_8_7_0/lib/libvips.so.42(vips_init+0x1b5)[0x7fafb9479dd5]
/vips_8_7_0/lib/libvips.so.42(vips_check_init+0x10)[0x7fafb9479fb0]
//vips_8_7_0/lib/libvips.so.42(vips_image_new_from_buffer+0x65)[0x7fafb94670a5]

you can reproduce it with the code given in #1414 , with a multiple iterations

[jcupitt]

Hi, thanks for report.

I built git master libvips like this:

$ CFLAGS="-g -Wall" CXXFLAGS="-g -Wall" ./autogen.sh --prefix=/home/john/vips --without-heif
$ make V=0 install

I tried this test program:

/*
 * Test.c
 *
 *  Created on: 27-Aug-2019
 *      Author: dinesh
 *
 * Compile with:
 * 	gcc -g -Wall dinesh.c `pkg-config vips --cflags --libs`
 */

#include <vips/vips.h>

static char* 
vipsThumbnail(char* inBuffer, size_t inBufferSize, size_t* outBufferSize)
{
	int width = 1024;
	int height = 1024;

	VipsImage *vipsImageOut;
	void *outBuffer;

	if(vips_thumbnail_buffer(inBuffer, inBufferSize, &vipsImageOut, width, 
		"height", height, 
		NULL))
		return NULL;

	if(vips_image_write_to_buffer(vipsImageOut, ".jpg", &outBuffer,
		outBufferSize, NULL)) {
		g_object_unref(vipsImageOut);
		return NULL;
	}

	g_object_unref(vipsImageOut);

	return outBuffer;
}

int 
main(int argc, char** argv)
{
	size_t inBufferSize;
	char* inBuffer;
	size_t outBufferSize;
	char* outBuffer;

	if(VIPS_INIT(argv[0]))
		vips_error_exit("error!!");

	vips_leak_set(TRUE);

	if(!g_file_get_contents(argv[1], &inBuffer, &inBufferSize, NULL))
		vips_error_exit("error!!");

	if(!(outBuffer = 
		vipsThumbnail(inBuffer, inBufferSize, &outBufferSize)))
		vips_error_exit("error!!");

	if(!g_file_set_contents(argv[2], outBuffer, outBufferSize, NULL))
		vips_error_exit("error!!");

	g_free(inBuffer);
	g_free(outBuffer);

	vips_shutdown();

	return 0;
}

And ran like this:

$ valgrind ./a.out ~/pics/Example1.heic x.png
==32643== Memcheck, a memory error detector
==32643== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==32643== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==32643== Command: ./a.out /home/john/pics/Example1.heic x.png
==32643== 
a.out: error!!
VipsForeignLoad: buffer is not in a known format
memory: high-water mark 0 bytes
error buffer: VipsForeignLoad: buffer is not in a known format
==32643== 
==32643== HEAP SUMMARY:
==32643==     in use at exit: 2,100,637 bytes in 13,233 blocks
==32643==   total heap usage: 27,792 allocs, 14,559 frees, 4,889,227 bytes allocated
==32643== 
==32643== LEAK SUMMARY:
==32643==    definitely lost: 0 bytes in 0 blocks
==32643==    indirectly lost: 0 bytes in 0 blocks
==32643==      possibly lost: 1,880 bytes in 24 blocks
==32643==    still reachable: 1,983,493 bytes in 12,394 blocks
==32643==                       of which reachable via heuristic:
==32643==                         newarray           : 1,536 bytes in 16 blocks
==32643==         suppressed: 0 bytes in 0 blocks
==32643== Rerun with --leak-check=full to see details of leaked memory
==32643== 
==32643== For counts of detected and suppressed errors, rerun with: -v
==32643== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

So I think (hope) it's OK.

What version did you try, what platform, what test image, exactly what test code did you run, etc.

[dineshkannaa]

It is occuring sometimes , not always . Im not usingvips_error_exit("error!!") as I have to return the buffer to Java and generate thumbnail for next images .I think this may be the reason . is it ok to avoid vips_error_exit("error!!") ?

[jcupitt]

No, you don't need to call vips_error_exit().

I think you are almost certainly seeing a bug in your Java bindings. Are you perhaps freeing the buffer twice?

[dineshkannaa]

Thank you . Will check on my side .

[dineshkannaa]

Hi , Identified the issue , Im copying vips_error_buffer() to my char pointer of predefined size . My char pointer overflowed as I didnt call vips_error_clear() on error cases . My process continuosly restarts due to this as Vips library is a single thread and vips_error_buffer() already has error string . Is it possible to call vips_error_clear through command line or I have to reinstall libvips .

[jcupitt]

Hmm I guess there is a race there, you're right. I've added vips_error_buffer_copy() to fix this. Thanks for pointing this out.

In the meantime, don't copy to a fixed-size char pointer without strncpy().

You could also use strdup(), for example:

char *message;

message = g_strdup (vips_error_buffer ());
vips_error_clear ();
do_something (message);
g_free (message);

[jcupitt]

Oh, or the strncpy() version:

char message[256];

strncpy (message, vips_error_buffer(), 256);
vips_error_clear();

I wouldn't call vips_error_buffer() from a worker thread. It supposed to be used by a top level thread to report logs back to the user. It's not a per-thread error mechanism.

[dineshkannaa]

Thank you . Is there anyway to reset vips ? I tried calling vips_error_clear() before copying the error buffer to solve the problem and applied patch . but it is not working . The process crashes and restarts again and again

[jcupitt]

Don't patch libvips, instead do the copy safely in your code.

[dineshkannaa]

I didnt patch in libvips , I patched my code in the same running thread where my buffer overflowed . like this

 char *errorString = (char*) malloc(10000 * sizeof(char));
 vips_error_clear();
strcat(errorString, (char*) vips_error_buffer());

but it didnt solve my problem . The process restarts again and again .

[jcupitt]

Don't use strcat()! It's very unsafe, plus you need to init the destination first.

Use the g_strdup() example above, it's very simple and safe.