lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.08k stars 425 forks source link

Veracode discovered a race condition #183

Open az-faro opened 1 year ago

az-faro commented 1 year ago

I use lodepng in a project that uses Veracode and it found the following issue:

Description: This call to fopen() results in a time-of-check time-of-use race condition in an earlier call to fopen() in function: lodepng_filesize(). This condition occurs when a given resource is checked and a change occurs in the resource to invalidate the results of the check before that resource is used. One example of this behavior is when a file is replaced with a symbolic link to another file after the check but before the use.

Remediation: When performing a sequence of actions on a file, obtain a file handle to ensure that the same resource is being accessed each time. For example, a stat() followed by an fopen() could be replaced by an open() followed by an fstat(), where the fstat() call uses the file handle returned by open() instead of a filename.

What it means essentially is that when the lodepng_load_file function is called it first opens the file and checks the size, then closes it (in lodepng_filesize) and then makes some check depending on the size of the file. After that it opens it again (in lodepng_buffer_file) to load data into the malloc:ed buffer. The issue is that the file can be changed between being read the first and second times, invalidating the size check and lodepng_malloc call. As said by Veracode; make sure to not close the file in-between or someone might be able to start with a small file, then change it to a large file right before the lodepng_buffer_file gets called to produce a buffer overrun.