Skip to content

Commit

Permalink
tga: check for valid color ID in apply_colormap
Browse files Browse the repository at this point in the history 
A malformed colourmapped tga file could specify color IDs that are not
contained in the colourmap. Therefore we add some bounds checking to
ensure that we only use entries that actually exist.

We could completely give up on such files, but it's just as easy to fall
back to the first colour in the map in this case. However we can only
fall back to the first colour in the map IF the colourmap contains
at least one entry. Therefore we add an up-front check to verify that
colourmapped images actually do contain at least one entry.

Without this bounds-checking, it's possible to induce a heap-buffer-overflow
read in apply-colormap(), see ASAN output below:

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61900002257c at pc 0x000000564f99 bp 0x7ffe65fdb040 sp 0x7ffe65fdb038
READ of size 1 at 0x61900002257c thread T0
    #0 0x564f98 in apply_colormap /home/ahunt/git/gimp/plug-ins/common/file-tga.c:901:23
    #1 0x56411a in read_line /home/ahunt/git/gimp/plug-ins/common/file-tga.c:975:7
    #2 0x560648 in ReadImage /home/ahunt/git/gimp/plug-ins/common/file-tga.c:1202:15
    #3 0x55f4ee in load_image /home/ahunt/git/gimp/plug-ins/common/file-tga.c:647:11
    #4 0x5653ab in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/plug-ins/common/file-tga_fuzzer.c:69:17
    #5 0x461624 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
    #6 0x460b2a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3
    #7 0x462847 in fuzzer::Fuzzer::MutateAndTestOne() /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
    #8 0x4633d5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
    #9 0x451686 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
    #10 0x47b662 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #11 0x7fe76a5dc349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    #12 0x424a39 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

0x61900002257c is located 0 bytes to the right of 1020-byte region [0x619000022180,0x61900002257c)
allocated by thread T0 here:
    #0 0x52ca8d in malloc /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x7fe76b65ccf2 in g_malloc /home/ahunt/git/glib/_build/../glib/gmem.c:106:13
    #2 0x55fdc6 in ReadImage /home/ahunt/git/gimp/plug-ins/common/file-tga.c:1049:26
    #3 0x55f4ee in load_image /home/ahunt/git/gimp/plug-ins/common/file-tga.c:647:11
    #4 0x5653ab in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/plug-ins/common/file-tga_fuzzer.c:69:17
    #5 0x461624 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
    #6 0x460b2a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3
    #7 0x462847 in fuzzer::Fuzzer::MutateAndTestOne() /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
    #8 0x4633d5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
    #9 0x451686 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
    #10 0x47b662 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #11 0x7fe76a5dc349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ahunt/git/gimp/plug-ins/common/file-tga.c:901:23 in apply_colormap
Shadow bytes around the buggy address:
  0x0c327fffc450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fffc460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fffc470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fffc480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fffc490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c327fffc4a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[04]
  0x0c327fffc4b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffc4c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffc4d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffc4e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffc4f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==16309==ABORTING

( crash-f70628d4df7a65babc8e57d890425771a1d67e06 )
  • Loading branch information
@ahunt @Wormnest
ahunt authored and Wormnest committed Jul 19, 2024
1 parent 49755f0 commit 723d383
Showing 1 changed file with 23 additions and 24 deletions.
47 changes: 23 additions & 24 deletions plug-ins/common/file-tga.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ load_image (GFile *file,
switch (info.imageType)
{
case TGA_TYPE_MAPPED:
if (info.bpp != 8)
if (info.bpp != 8 || !info.colorMapLength)
{
g_message ("Unhandled sub-format in '%s' (type = %u, bpp = %u)",
gimp_file_get_utf8_name (file),
Expand Down Expand Up @@ -862,32 +862,31 @@ apply_colormap (guchar *dest,
guint width,
const guchar *cmap,
gboolean alpha,
guint16 index)
guint16 colorMapIndex,
guint16 colorMapLength)
{
guint x;

if (alpha)
{
for (x = 0; x < width; x++)
{
*(dest++) = cmap[(*src - index) * 4];
*(dest++) = cmap[(*src - index) * 4 + 1];
*(dest++) = cmap[(*src - index) * 4 + 2];
*(dest++) = cmap[(*src - index) * 4 + 3];

src++;
}
}
else
for (x = 0; x < width; x++)
{
for (x = 0; x < width; x++)
{
*(dest++) = cmap[(*src - index) * 3];
*(dest++) = cmap[(*src - index) * 3 + 1];
*(dest++) = cmap[(*src - index) * 3 + 2];

src++;
}
guchar entryIndex = src[x] - colorMapIndex;

if (src[x] < colorMapIndex || entryIndex >= colorMapLength) {
g_message ("Unsupported colormap entry: %u",
src[x]);
entryIndex = 0;
}

if (alpha) {
*(dest++) = cmap[entryIndex * 4];
*(dest++) = cmap[entryIndex * 4 + 1];
*(dest++) = cmap[entryIndex * 4 + 2];
*(dest++) = cmap[entryIndex * 4 + 3];
} else {
*(dest++) = cmap[entryIndex * 3];
*(dest++) = cmap[entryIndex * 3 + 1];
*(dest++) = cmap[entryIndex * 3 + 2];
}
}
}

Expand Down Expand Up @@ -943,7 +942,7 @@ read_line (FILE *fp,
gboolean has_alpha = (info->alphaBits > 0);

apply_colormap (row, buf, info->width, convert_cmap, has_alpha,
info->colorMapIndex);
info->colorMapIndex, info->colorMapLength);
}
else if (info->imageType == TGA_TYPE_MAPPED)
{
Expand Down

0 comments on commit 723d383

Please sign in to comment.