CVE-2016-6906: Out-of-bounds Read (CWE-125)
value: The read_image_tga function in gd_tga.c in the GD Graphics Library (aka libgd) before 2.2.4 allows remote attackers to cause a denial of service (out-of-bounds read) via a crafted TGA file, related to the decompression buffer.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2017-03-15T14:59Z
Repository: https://github.com/libgd/libgd
Language: C
Files changed: src/gd_tga.c, tests/tga/.gitignore, tests/tga/CMakeLists.txt, tests/tga/Makemodule.am, tests/tga/heap_overflow.c
Commit date: 2016-12-13 13:31:05 +0100
Commit message: Fix OOB reads of the TGA decompression buffer

It is possible to craft TGA files which will overflow the decompression
buffer, but not the image's bitmap. Therefore we augment the check for the
bitmap's overflow with a check for the buffer's overflow.

This issue had been reported by Ibrahim El-Sayed to security@libgd.org.

CVE-2016-6906

Diff: diff --git a/src/gd_tga.c b/src/gd_tga.c
index 8737b045..68e4b17d 100644
--- a/src/gd_tga.c
+++ b/src/gd_tga.c
@@ -298,11 +298,12 @@ int read_image_tga( gdIOCtx *ctx, oTga *tga )
 			
 			if ((decompression_buffer[buffer_caret] & TGA_RLE_FLAG) == TGA_RLE_FLAG) {
 				encoded_pixels = ( ( decompression_buffer[ buffer_caret ] & ~TGA_RLE_FLAG ) + 1 );
 				buffer_caret++;
 
-				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size) {
+				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size
+						|| buffer_caret + pixel_block_size > rle_size) {
 					gdFree( decompression_buffer );
 					gdFree( conversion_buffer );
 					return -1;
 				}
 
@@ -314,11 +315,12 @@ int read_image_tga( gdIOCtx *ctx, oTga *tga )
 
 			} else {
 				encoded_pixels = decompression_buffer[ buffer_caret ] + 1;
 				buffer_caret++;
 
-				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size) {
+				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size
+						|| buffer_caret + (encoded_pixels * pixel_block_size) > rle_size) {
 					gdFree( decompression_buffer );
 					gdFree( conversion_buffer );
 					return -1;
 				}
 
diff --git a/tests/tga/.gitignore b/tests/tga/.gitignore
index 7a659b14..cf0556b5 100644
--- a/tests/tga/.gitignore
+++ b/tests/tga/.gitignore
@@ -1,7 +1,8 @@
 /bug00084
 /bug00247
 /bug00247a
 /bug00248
 /bug00248a
+/heap_overflow
 /tga_null
 /tga_read
diff --git a/tests/tga/CMakeLists.txt b/tests/tga/CMakeLists.txt
index 789fb146..11542a03 100644
--- a/tests/tga/CMakeLists.txt
+++ b/tests/tga/CMakeLists.txt
@@ -3,9 +3,10 @@ LIST(APPEND TESTS_FILES
 	bug00084
 	bug00247
 	bug00247a
 	bug00248
 	bug00248a
+	heap_overflow
 	tga_read
 )
 
 ADD_GD_TESTS()
diff --git a/tests/tga/Makemodule.am b/tests/tga/Makemodule.am
index a1e6af60..916d7073 100644
--- a/tests/tga/Makemodule.am
+++ b/tests/tga/Makemodule.am
@@ -2,18 +2,20 @@ libgd_test_programs += \
 	tga/bug00084 \
 	tga/bug00247 \
 	tga/bug00247a \
 	tga/bug00248 \
 	tga/bug00248a \
+	tga/heap_overflow \
 	tga/tga_null \
 	tga/tga_read
 
 EXTRA_DIST += \
 	tga/CMakeLists.txt \
 	tga/bug00084.tga \
 	tga/bug00247.tga \
 	tga/bug00247a.tga \
 	tga/bug00248.tga \
 	tga/bug00248a.tga \
+	tga/heap_overflow.tga \
 	tga/tga_read_rgb.png \
 	tga/tga_read_rgb.tga \
 	tga/tga_read_rgb_rle.tga
diff --git a/tests/tga/heap_overflow.c b/tests/tga/heap_overflow.c
new file mode 100644
index 00000000..0e9a2d00
--- /dev/null
+++ b/tests/tga/heap_overflow.c
@@ -0,0 +1,51 @@
+/**
+ * Test that the crafted TGA file doesn't trigger OOB reads.
+ */
+
+
+#include "gd.h"
+#include "gdtest.h"
+
+
+static size_t read_test_file(char **buffer, char *basename);
+
+
+int main()
+{
+    gdImagePtr im;
+    char *buffer;
+    size_t size;
+
+    size = read_test_file(&buffer, "heap_overflow.tga");
+    im = gdImageCreateFromTgaPtr(size, (void *) buffer);
+    gdTestAssert(im == NULL);
+    free(buffer);
+
+    return gdNumFailures();
+}
+
+
+static size_t read_test_file(char **buffer, char *basename)
+{
+    char *filename;
+    FILE *fp;
+    size_t exp_size, act_size;
+
+    filename = gdTestFilePath2("tga", basename);
+    fp = fopen(filename, "rb");
+    gdTestAssert(fp != NULL);
+
+	fseek(fp, 0, SEEK_END);
+	exp_size = ftell(fp);
+	fseek(fp, 0, SEEK_SET);
+
+    *buffer = malloc(exp_size);
+    gdTestAssert(*buffer != NULL);
+    act_size = fread(*buffer, sizeof(**buffer), exp_size, fp);
+    gdTestAssert(act_size == exp_size);
+
+    fclose(fp);
+    free(filename);
+
+    return act_size;
+}
diff --git a/tests/tga/heap_overflow.tga b/tests/tga/heap_overflow.tga
new file mode 100644
index 00000000..e9bc0ecb
Binary files /dev/null and b/tests/tga/heap_overflow.tga differ

Vulnerable code: 				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size) {
				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size) {
Fixed code: 				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size
						|| buffer_caret + pixel_block_size > rle_size) {
				if ((bitmap_caret + (encoded_pixels * pixel_block_size)) > image_block_size
						|| buffer_caret + (encoded_pixels * pixel_block_size) > rle_size) {
/heap_overflow
	heap_overflow
	tga/heap_overflow \
	tga/heap_overflow.tga \
/**
 * Test that the crafted TGA file doesn't trigger OOB reads.
 */


#include "gd.h"
#include "gdtest.h"


static size_t read_test_file(char **buffer, char *basename);


int main()
{
    gdImagePtr im;
    char *buffer;
    size_t size;

    size = read_test_file(&buffer, "heap_overflow.tga");
    im = gdImageCreateFromTgaPtr(size, (void *) buffer);
    gdTestAssert(im == NULL);
    free(buffer);

    return gdNumFailures();
}


static size_t read_test_file(char **buffer, char *basename)
{
    char *filename;
    FILE *fp;
    size_t exp_size, act_size;

    filename = gdTestFilePath2("tga", basename);
    fp = fopen(filename, "rb");
    gdTestAssert(fp != NULL);

	fseek(fp, 0, SEEK_END);
	exp_size = ftell(fp);
	fseek(fp, 0, SEEK_SET);

    *buffer = malloc(exp_size);
    gdTestAssert(*buffer != NULL);
    act_size = fread(*buffer, sizeof(**buffer), exp_size, fp);
    gdTestAssert(act_size == exp_size);

    fclose(fp);
    free(filename);

    return act_size;
}