Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Libzip example contains uninitialised values when checked with Valgrind

Tags:

c++

zip

valgrind

I've been using libzip to handle zip files, and have based my code on the example found in rodrigo's answer to this question. Here's his code, for quick reference:

#include <zip.h>

int main()
{
    //Open the ZIP archive
    int err = 0;
    zip *z = zip_open("foo.zip", 0, &err);

    //Search for the file of given name
    const char *name = "file.txt";
    struct zip_stat st;
    zip_stat_init(&st);
    zip_stat(z, name, 0, &st);

    //Alloc memory for its uncompressed contents
    char *contents = new char[st.size];

    //Read the compressed file
    zip_file *f = zip_fopen(z, "file.txt", 0);
    zip_fread(f, contents, st.size);
    zip_fclose(f);

    //And close the archive
    zip_close(z);
}

I traced the errors I subsequently got from Valgrind back to this code- it complains of an uninitialised value when opening the zipped "file.txt" using 'zip_fopen()`.

==29256== Conditional jump or move depends on uninitialised value(s)
==29256==    at 0x5B4B290: inflateReset2 (in /usr/lib/libz.so.1.2.3.4)
==29256==    by 0x5B4B37F: inflateInit2_ (in /usr/lib/libz.so.1.2.3.4)
==29256==    by 0x4E2EB8C: zip_fopen_index (in /usr/lib/libzip.so.1.0.0)
==29256==    by 0x400C32: main (main.cpp:24)
==29256==  Uninitialised value was created by a heap allocation
==29256==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==29256==    by 0x5B4B35B: inflateInit2_ (in /usr/lib/libz.so.1.2.3.4)
==29256==    by 0x4E2EB8C: zip_fopen_index (in /usr/lib/libzip.so.1.0.0)
==29256==    by 0x400C32: main (main.cpp:24)
==29256==
==29256==
==29256== HEAP SUMMARY:
==29256==     in use at exit: 71 bytes in 1 blocks
==29256==   total heap usage: 26 allocs, 25 frees, 85,851 bytes allocated
==29256==
==29256== 71 bytes in 1 blocks are definitely lost in loss record 1 of 1
==29256==    at 0x4C24A72: operator new[](unsigned long) (vg_replace_malloc.c:305)
==29256==    by 0x400BEE: main (main.cpp:19)

I can't see where the uninitialised value is coming from in this code. Can anyone trace this, or does the fault lie with libzip itself? Should I switch to another zip library- for example, Minizip?

EDIT: The 71 bytes are the contents of file.txt which was read in- delete[] contents; tagged on the end will eliminate that.

(I'd have left a comment on the original answer to draw attention to the issue, but I do not have the requisite rep.)

like image 458
Sam Finnigan Avatar asked Sep 18 '12 19:09

Sam Finnigan


1 Answers

You made me look :)

Yes, it's an error inside zlib (used by libzip), since both the allocation and the use of the memory is inside inflateInit2_ on the same call. Your code doesn't even have a chance to get to that memory.

I can repeat the problem using zlib 1.2.3, but it's not showing up anymore in 1.2.7. I didn't have the code for 1.2.3 available, but if you're looking at it, I'd check the initialization of state and how it's used inside inflateReset2.

Edit: Tracked down the problem, I downloaded Ubuntu's source package for zlib (1.2.3.4) and the offending line is;

if (state->wbits != windowBits && state->window != Z_NULL) {

wbits is not initialized previous to this, and will cause the warning. The odd thing is that neither the original zlib 1.2.3 or 1.2.4 have this problem, it seems to be unique for Ubuntu. 1.2.3 doesn't even have the function inflateReset2, and 1.2.4 has it right;

if (state->window != Z_NULL && state->wbits != (unsigned)windowBits) {

since window is previously initialized to Z_NULL, the uninitialized wbits read won't happen.

like image 155
Joachim Isaksson Avatar answered Oct 05 '22 03:10

Joachim Isaksson