RandyGaul / cute_headers

Collection of cross-platform one-file C/C++ libraries with no dependencies, primarily used for games
4.24k stars 264 forks source link

cute_png segfaults on PNG with header saying it's 1000000x1000000 pixels #335

Closed dbohdan closed 1 year ago

dbohdan commented 1 year ago

If you alter a PNG file's header to say it has an extremely high resolution, cute_png can segfault while attempting to load it. I haven't tested if the same thing happens with valid images of similar resolution.

Here is a test case. When I build it with -static and run it directly or build it with or without -static and run it with Valgrind, I get SIGSEGV.

> gcc -static -o test test.c && ./test
fish: Job 1, './test' terminated by signal SIGSEGV (Address boundary error)
#define CUTE_PNG_IMPLEMENTATION
#include "cute_png.h"
#include <stdio.h>

int main() {
  // The header of this image says it is 1000000x1000000.
  unsigned char wrong_size_png[] = {
      0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d,
      0x49, 0x48, 0x44, 0x52, 0x00, 0x0f, 0x42, 0x40, 0x00, 0x0f, 0x42, 0x40,
      0x08, 0x06, 0x00, 0x00, 0x00, 0x5c, 0x6d, 0x38, 0x7d, 0x00, 0x00, 0x00,
      0x2b, 0x49, 0x44, 0x41, 0x54, 0x58, 0xc3, 0xed, 0xce, 0x31, 0x01, 0x00,
      0x00, 0x08, 0xc0, 0xa0, 0xf5, 0x2f, 0xad, 0x31, 0xf4, 0x80, 0x04, 0x54,
      0x4d, 0x0f, 0x48, 0x48, 0x48, 0x48, 0x48, 0x48, 0x48, 0x48, 0x48, 0x48,
      0x48, 0x48, 0x48, 0x48, 0x48, 0x9c, 0x25, 0x16, 0xed, 0x50, 0x1f, 0xe1,
      0x75, 0x20, 0xa1, 0x23, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44,
      0xae, 0x42, 0x60, 0x82};
  unsigned int wrong_size_png_len = 100;

  cp_image_t img = cp_load_png_mem(wrong_size_png, wrong_size_png_len);
  if (img.pix == 0) {
    fprintf(stderr, "bad PNG\n");
  }
}

Here is the output from Valgrind:

==746316== Memcheck, a memory error detector
==746316== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==746316== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==746316== Command: ./test
==746316== 
==746316== Syscall param set_robust_list(head) points to uninitialised byte(s)
==746316==    at 0x459D0A: __tls_init_tp (in .../test)
==746316==    by 0x4093C9: __libc_setup_tls (in .../test)
==746316==    by 0x408525: (below main) (in .../test)
==746316==  Address 0x40006b0 is in the brk data segment 0x4000000-0x4000dbf
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x46C26E: getrandom (in .../test)
==746316==    by 0x423AD5: ptmalloc_init.part.0 (in .../test)
==746316==    by 0x4268C4: malloc (in .../test)
==746316==    by 0x48654A: _dl_get_origin (in .../test)
==746316==    by 0x45B006: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266D1: malloc (in .../test)
==746316==    by 0x48654A: _dl_get_origin (in .../test)
==746316==    by 0x45B006: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4267C1: malloc (in .../test)
==746316==    by 0x48654A: _dl_get_origin (in .../test)
==746316==    by 0x45B006: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x425461: _int_malloc (in .../test)
==746316==    by 0x425F74: tcache_init.part.0 (in .../test)
==746316==    by 0x4267CB: malloc (in .../test)
==746316==    by 0x48654A: _dl_get_origin (in .../test)
==746316==    by 0x45B006: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x48654A: _dl_get_origin (in .../test)
==746316==    by 0x45B006: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x483030: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x48304D: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x483309: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x44C520: __strcspn_sse42 (in .../test)
==746316==    by 0x48DCCD: strsep (in .../test)
==746316==    by 0x482C1E: fillin_rpath.isra.0 (in .../test)
==746316==    by 0x483334: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x44C4FA: __strcspn_sse42 (in .../test)
==746316==    by 0x48DCCD: strsep (in .../test)
==746316==    by 0x482C1E: fillin_rpath.isra.0 (in .../test)
==746316==    by 0x483334: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x428D1E: strdup (in .../test)
==746316==    by 0x482C41: fillin_rpath.isra.0 (in .../test)
==746316==    by 0x483334: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x482D54: fillin_rpath.isra.0 (in .../test)
==746316==    by 0x483334: _dl_init_paths (in .../test)
==746316==    by 0x45B733: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x48F9AE: _dl_find_object_init (in .../test)
==746316==    by 0x45B987: _dl_non_dynamic_init (in .../test)
==746316==    by 0x45C9D9: __libc_init_first (in .../test)
==746316==    by 0x4085C9: (below main) (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x4042EE: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316== 
==746316== Warning: set address range perms: large range [0x4800000, 0x57515000) (defined)
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4266EE: malloc (in .../test)
==746316==    by 0x4044B7: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x427B36: calloc (in .../test)
==746316==    by 0x4029BC: cp_inflate (in .../test)
==746316==    by 0x4046A2: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x427BE8: calloc (in .../test)
==746316==    by 0x4029BC: cp_inflate (in .../test)
==746316==    by 0x4046A2: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316== 
==746316== Conditional jump or move depends on uninitialised value(s)
==746316==    at 0x4231E4: _int_free (in .../test)
==746316==    by 0x426A40: free (in .../test)
==746316==    by 0x402C11: cp_inflate (in .../test)
==746316==    by 0x4046A2: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316== 
==746316== Invalid read of size 1
==746316==    at 0x403A89: cp_unfilter (in .../test)
==746316==    by 0x4046D0: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316==  Address 0x57783d6c is not stack'd, malloc'd or (recently) free'd
==746316== 
==746316== 
==746316== Process terminating with default action of signal 11 (SIGSEGV)
==746316==  Access not within mapped region at address 0x57783D6C
==746316==    at 0x403A89: cp_unfilter (in .../test)
==746316==    by 0x4046D0: cp_load_png_mem (in .../test)
==746316==    by 0x406997: main (in .../test)
==746316==  If you believe this happened as a result of a stack
==746316==  overflow in your program's main thread (unlikely but
==746316==  possible), you can try to increase the size of the
==746316==  main thread stack using the --main-stacksize= flag.
==746316==  The main thread stack size used in this run was 8388608.
==746316== 
==746316== HEAP SUMMARY:
==746316==     in use at exit: 0 bytes in 0 blocks
==746316==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==746316== 
==746316== All heap blocks were freed -- no leaks are possible
==746316== 
==746316== Use --track-origins=yes to see where uninitialised values come from
==746316== For lists of detected and suppressed errors, rerun with: -s
==746316== ERROR SUMMARY: 20 errors from 20 contexts (suppressed: 0 from 0)
RandyGaul commented 1 year ago

Thanks for the note, did you happen to also find a suggested fix?

dbohdan commented 1 year ago

Fixed via #336.