Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potentially undefined variables #18

Open
michielp1807 opened this issue Dec 5, 2021 · 1 comment
Open

Potentially undefined variables #18

michielp1807 opened this issue Dec 5, 2021 · 1 comment

Comments

@michielp1807
Copy link
Contributor

Compiler warnings

When compiling with gcc -Wall -O3, the compiler gives quite a few warnings. A lot of these consist of the fact that the return value of read() is often ignored, which is probably not really a problem. There are also some warnings about potentially undefined variables:

  • The fact that str_len could be uninitialized is probably a false positive since I think key starts out as equal to clear:
./gifdec.c:369:45: warning: ‘str_len’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  369 |             ret = add_entry(&table, str_len + 1, key, entry.suffix);
  • I'm not quite sure what causes the following warning about entry.suffix:
./gifdec.c:271:37: warning: ‘entry.suffix’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  271 |     table->entries[table->nentries] = (Entry) {length, prefix, suffix};
  • table_is_full could probably be initialized to 0:
./gifdec.c:368:19: warning: ‘table_is_full’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  368 |         } else if (!table_is_full) {

Valgrind warnings

When fuzzing with AFL, we found a couple of gifs that crash the library. When analyzing these crashes with Valgrind, we get warnings for conditional jumps or moves depending on uninitialized values and also the usage of uninitialized values.

Here are two gifs that currently crash the library:

  • segfault.gif results in a Segmentation fault (core dumped) crash
  • double_free_or_corruption.gif results in a double free or corruption (out) Aborted (core dumped) crash

(Right click and save image as to download, these are not proper working gifs so your browser probably won't display them.)

Using Valgrind with --track-origins=yes we get the following warnings for segfault.gif:

==8703== Conditional jump or move depends on uninitialised value(s)
==8703==    at 0x10A77A: read_image_data (gifdec.c:368)
==8703==    by 0x10A77A: read_image (gifdec.c:441)
==8703==    by 0x10A77A: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a stack allocation
==8703==    at 0x109B81: gd_get_frame (gifdec.c:487)
==8703== Conditional jump or move depends on uninitialised value(s)
==8703==    at 0x10A7E1: read_image_data (gifdec.c:385)
==8703==    by 0x10A7E1: read_image (gifdec.c:441)
==8703==    by 0x10A7E1: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a heap allocation
==8703==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8703==    by 0x10A2D7: new_table (gifdec.c:245)
==8703==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8703==    by 0x10A2D7: read_image (gifdec.c:441)
==8703==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703== Conditional jump or move depends on uninitialised value(s)
==8703==    at 0x10AA8D: read_image_data (gifdec.c:363)
==8703==    by 0x10AA8D: read_image (gifdec.c:441)
==8703==    by 0x10AA8D: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a heap allocation
==8703==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8703==    by 0x10A2D7: new_table (gifdec.c:245)
==8703==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8703==    by 0x10A2D7: read_image (gifdec.c:441)
==8703==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703== Conditional jump or move depends on uninitialised value(s)
==8703==    at 0x10A8D1: interlaced_line_index (gifdec.c:315)
==8703==    by 0x10A8D1: read_image_data (gifdec.c:390)
==8703==    by 0x10A8D1: read_image (gifdec.c:441)
==8703==    by 0x10A8D1: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a heap allocation
==8703==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8703==    by 0x10A2D7: new_table (gifdec.c:245)
==8703==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8703==    by 0x10A2D7: read_image (gifdec.c:441)
==8703==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703== Use of uninitialised value of size 8
==8703==    at 0x10A868: read_image_data (gifdec.c:391)
==8703==    by 0x10A868: read_image (gifdec.c:441)
==8703==    by 0x10A868: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a heap allocation
==8703==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8703==    by 0x10A2D7: new_table (gifdec.c:245)
==8703==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8703==    by 0x10A2D7: read_image (gifdec.c:441)
==8703==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703== Conditional jump or move depends on uninitialised value(s)
==8703==    at 0x10AA71: read_image_data (gifdec.c:398)
==8703==    by 0x10AA71: read_image (gifdec.c:441)
==8703==    by 0x10AA71: gd_get_frame (gifdec.c:500)
==8703==    by 0x109494: main (test.c:45)
==8703==  Uninitialised value was created by a stack allocation
==8703==    at 0x109B81: gd_get_frame (gifdec.c:487)

And for double_free_or_corruption.gif we get some of the same warnings as above, but also:

==8729== Use of uninitialised value of size 8
==8729==    at 0x109692: memcpy (string_fortified.h:34)
==8729==    by 0x109692: render_frame_rect (gifdec.c:455)
==8729==    by 0x109423: main (test.c:53)
==8729==  Uninitialised value was created by a heap allocation
==8729==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8729==    by 0x10A2D7: new_table (gifdec.c:245)
==8729==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8729==    by 0x10A2D7: read_image (gifdec.c:441)
==8729==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8729==    by 0x109402: main (test.c:45)
==8729== Conditional jump or move depends on uninitialised value(s)
==8729==    at 0x10968C: render_frame_rect (gifdec.c:454)
==8729==    by 0x109423: main (test.c:53)
==8729==  Uninitialised value was created by a heap allocation
==8729==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8729==    by 0x10A2D7: new_table (gifdec.c:245)
==8729==    by 0x10A2D7: read_image_data (gifdec.c:355)
==8729==    by 0x10A2D7: read_image (gifdec.c:441)
==8729==    by 0x10A2D7: gd_get_frame (gifdec.c:500)
==8729==    by 0x109402: main (test.c:45)

I haven't looked into what causes these warnings/errors, but it turns out that it possible to crash the library with corrupted gifs because of potentially uninitialized variables.

@BoltSwith
Copy link

I am getting similar compiler warnings for same variables (in same functions) as mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants