[gimp] gif: don't use strlcpy on non-null terminated string

[Jehan <jehanp src gnome org>]

commit 5828e0a12de77bc2e6a847678673190a52a2105c
Author: Andrzej Hunt <andrzej ahunt org>
Date:   Fri Sep 10 19:01:03 2021 +0200

    gif: don't use strlcpy on non-null terminated string
    
    strlcpy() only copies the specified number of bytes, but it also iterates over
    the input string in order to return its length. We only populate the first
    6 bytes of buf - the rest is uninitialised - hence strlcpy() will iterate
    past the 3 bytes that we're trying to compare, and will only return after
    reading an effectively random number of bytes (which could well be beyond
    the end of buf). Since we ignore strlcpy()'s return value, program flow
    is generally not affected (unless we hit a segfault I guess, which is unlikely
    because we do 0-initialize some of our stack variables, meaning strlcpy() is
    unlikely to go far). However, this iteration is wasteful, and triggers
    complaints from code sanitizers.
    
    Therefore we remove the strlcpy,() and switch to doing a strncmp() on just the
    3 bytes that we care about (similar to what is done for the "GIF" check above).
    This change has the nice benefit of avoiding an unneeded copy.
    
    An example implementation of strlcpy can be found in glib, here's the line
    where it iterates until the first NULL byte:
      
https://gitlab.gnome.org/GNOME/glib/-/blob/f763f2b7cb65499b4fd8a6a4922ce375ef078ca3/glib/gstrfuncs.c#L1484
    
    See also ASAN output from when this was discovered below:
    
    ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc776e03d0 at pc 0x7f909b615ba2 bp 
0x7ffc776e0370 sp 0x7ffc776e0368
    READ of size 1 at 0x7ffc776e03d0 thread T0
        #0 0x7f909b615ba1 in g_strlcpy /home/ahunt/git/glib/_build/../glib/gstrfuncs.c:1484:14
        #1 0x55dc5c in load_image /home/ahunt/git/gimp/plug-ins/common/file-gif-load.c:419:3
        #2 0x561f20 in LLVMFuzzerTestOneInput 
/home/ahunt/git/gimp/plug-ins/common/file-gif-load_fuzzer.c:79:17
        #3 0x460e44 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
        #4 0x46034a 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
        #5 0x462067 in fuzzer::Fuzzer::MutateAndTestOne() 
/home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
        #6 0x462bf5 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
        #7 0x450ea6 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
        #8 0x47ae82 in main 
/home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
        #9 0x7f909a564349 in __libc_start_main (/lib64/libc.so.6+0x24349)
        #10 0x424259 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120
    
    Address 0x7ffc776e03d0 is located in stack of thread T0 at offset 48 in frame
        #0 0x55d89f in load_image /home/ahunt/git/gimp/plug-ins/common/file-gif-load.c:375
    
      This frame has 6 object(s):
        [32, 48) 'buf' (line 378) <== Memory access at offset 48 overflows this variable
        [64, 65) 'c' (line 379)
        [80, 848) 'localColorMap' (line 380)
        [976, 980) 'grayScale' (line 381)
        [992, 996) 'version' (line 385)
        [1008, 1016) 'image' (line 386)
    HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext 
or vfork
          (longjmp and C++ exceptions *are* supported)
    SUMMARY: AddressSanitizer: stack-buffer-overflow /home/ahunt/git/glib/_build/../glib/gstrfuncs.c:1484:14 
in g_strlcpy
    Shadow bytes around the buggy address:
      0x10000eed4020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed4030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed4040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed4050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed4060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    =>0x10000eed4070: 00 00 00 00 f1 f1 f1 f1 00 00[f2]f2 01 f2 00 00
      0x10000eed4080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed4090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed40a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed40b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x10000eed40c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    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
    ==15243==ABORTING

 plug-ins/common/file-gif-load.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
---
diff --git a/plug-ins/common/file-gif-load.c b/plug-ins/common/file-gif-load.c
index a10438482e..22adc63460 100644
--- a/plug-ins/common/file-gif-load.c
+++ b/plug-ins/common/file-gif-load.c
@@ -389,7 +389,6 @@ load_image (GFile     *file,
   gboolean   useGlobalColormap;
   gint       bitPixel;
   gint       imageCount = 0;
-  gchar      version[4];
   GimpImage *image      = NULL;
   gboolean   status;
 
@@ -421,9 +420,7 @@ load_image (GFile     *file,
       return NULL;
     }
 
-  g_strlcpy (version, (gchar *) buf + 3, 4);
-
-  if ((strcmp (version, "87a") != 0) && (strcmp (version, "89a") != 0))
+  if ((strncmp (buf + 3, "87a", 3) != 0) && (strncmp (buf + 3, "89a", 3) != 0))
     {
       g_message ("Bad version number, not '87a' or '89a'");
       fclose (fd);