netmail-open / wjelement

advanced, flexible JSON manipulation in C
GNU Lesser General Public License v3.0
108 stars 56 forks source link

Valgrind complains about uninitialized memory access #79

Closed RelayrMA closed 5 years ago

RelayrMA commented 5 years ago

Got the following valgrind trace:

==26413== Conditional jump or move depends on uninitialised value(s)
==26413==    at 0x4A0B0A9: strlen (vg_replace_strmem.c:458)
==26413==    by 0x8FD3E29: WJRFillBuffer (wjreader.c:234)
==26413==    by 0x8FD41A0: WJRDown (wjreader.c:316)
==26413==    by 0x8FD4C42: WJRNext (wjreader.c:822)
==26413==    by 0x6CF9016: _WJELoad (element.c:254)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF8FF2: _WJELoad (element.c:255)
==26413==    by 0x6CF920A: _WJEOpenDocument (element.c:328)
==26413==    by 0x6CF93EB: __WJEFromString (element.c:413)
==26413==    by 0x138AFB: validate_against_schema (protocol_schema_validator.c:58)
==26413==    by 0x137B2B: test_Agent_PerformTask_DownloadPackage (utest_agent_tasks.c:232)
==26413==    by 0x6AF0B98: cmocka_run_one_test_or_fixture (in /usr/lib64/libcmocka.so.0.4.1)
==26413==    by 0x6AF1480: _cmocka_run_group_tests (in /usr/lib64/libcmocka.so.0.4.1)
==26413==    by 0x1386AD: __wrap_main (utest_agent_tasks.c:313)
==26413==    by 0x6F29AE6: (below main) (libc-start.c:308)
==26413==  Uninitialised value was created by a heap allocation
==26413==    at 0x4A07EAF: malloc (vg_replace_malloc.c:299)
==26413==    by 0x93DD562: MemMallocEx (xpl.c:327)
==26413==    by 0x8FD43A8: _WJROpenDocument (wjreader.c:433)
==26413==    by 0x6CF93B8: __WJEFromString (element.c:412)
==26413==    by 0x138AFB: validate_against_schema (protocol_schema_validator.c:58)
==26413==    by 0x137B2B: test_Agent_PerformTask_DownloadPackage (utest_agent_tasks.c:232)
==26413==    by 0x6AF0B98: cmocka_run_one_test_or_fixture (in /usr/lib64/libcmocka.so.0.4.1)
==26413==    by 0x6AF1480: _cmocka_run_group_tests (in /usr/lib64/libcmocka.so.0.4.1)
==26413==    by 0x1386AD: __wrap_main (utest_agent_tasks.c:313)
==26413==    by 0x6F29AE6: (below main) (libc-start.c:308)
==26413== 

I traced it down to access to be before doc->read. I extended the memset in _WJROpenDocument() from sizeof(WJIReader) to sizeof(WJIReader) + maxdepth and it seems to work now.

penduin commented 5 years ago

@RelayrMA thanks for spotting this, and for the fix! I've put that change in. commit: 926d1e8115e1a44986bdec7944dd2c479e417cd3