valgrind error for modp_b16_encode (patch included)

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Use modp_b16_encode with dynamically allocated buffer of length not 
divisible by 4. (Example attached.)
2. Run with valgrind.

What is the expected output? What do you see instead?

Expect no valgrind errors.  See invalid read of size 4 error.

What version of the product are you using? On what operating system?

3.10.3 on OS X 10.7.

Please provide any additional information below.

The problem occurs because once modp_b16_encode has reached the end of its loop 
it does a final read of the next 4 bytes.  Or, if the size is < 4, the 
initialization of x reads 4 bytes.

   // modp_b16_encode.c: 58
   uint32_t x = *srcInt++;

  // modp_b16_encode.c: 80
  x = *srcInt++;

As the buffer contains 1, 2, or 3 remaining bytes, this overflows the buffer 
and causes the valgrind error.  The result is still correct because the 
remaining leftover handling code only looks at the bits of x that were in the 
buffer.  But, the valgrind error is easily avoidable and doing so is both 
satisfying and eases valgrinding of code that uses modp_b16_encode.

A patch is attached that fixes the issue in modp_b16_encode.  I expect that 
many of the other encoders have similar problems, however.

Original issue reported on code.google.com by calfe...@qualys.com on 28 Nov 2011 at 5:16

Attachments:

• bad_modp_b16_encode.c
• modp_b16.c.patch

[GoogleCodeExporter]

Sorry for the delay.

Thank you for a great bug report.

I'll set up a test case, have it fail, then add your patch.

Original comment by nickg@client9.com on 24 Feb 2012 at 5:24

• Changed state: Started

[GoogleCodeExporter]

Hi
so the example has a bug in it.  obuf should be length of  7, due to null byte 
needed ;-)

but confirmed the original problem with test case

"make valgrind" now fails

=test/modp_b16_test.c .........==45436== Invalid read of size 4
==45436==    at 0x196C5: modp_b16_encode (in 
/Users/ngalbreath/stringencoders/.libs/libmodpbase64.0.dylib)
==45436==    by 0x1000036D7: testValgrind (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436==    by 0x100003A6F: all_tests (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436==    by 0x100003AF9: main (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436==  Address 0x100009130 is 0 bytes inside a block of size 3 alloc'd
==45436==    at 0xB823: malloc (vg_replace_malloc.c:266)
==45436==    by 0x100003619: testValgrind (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436==    by 0x100003A6F: all_tests (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436==    by 0x100003AF9: main (in 
/Users/ngalbreath/stringencoders/.libs/modp_b16_test)
==45436== 
OK (9 tests)
==45436== 
==45436== HEAP SUMMARY:
==45436==     in use at exit: 6,133 bytes in 33 blocks
==45436==   total heap usage: 35 allocs, 2 frees, 6,143 bytes allocated
==45436== 
==45436== LEAK SUMMARY:
==45436==    definitely lost: 0 bytes in 0 blocks
==45436==    indirectly lost: 0 bytes in 0 blocks
==45436==      possibly lost: 0 bytes in 0 blocks
==45436==    still reachable: 6,133 bytes in 33 blocks
==45436==         suppressed: 0 bytes in 0 blocks
==45436== Reachable blocks (those to which a pointer was found) are not shown.
==45436== To see them, rerun with: --leak-check=full --show-reachable=yes
==45436== 
==45436== For counts of detected and suppressed errors, rerun with: -v
==45436== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 1 from 1)
make: *** [valgrind] Error 1

Original comment by nickg@client9.com on 25 Feb 2012 at 5:45

[GoogleCodeExporter]

$ svn ci -m 'Issue 21: Fixed valgrind complaints on modp_b16_encode.  Thanks  
calfeld@qualys.com' ChangeLog src/modp_b16.c 
Sending        ChangeLog
Sending        src/modp_b16.c
Transmitting file data ..
Committed revision 262.

Original comment by nickg@client9.com on 26 Feb 2012 at 4:11

• Changed state: Verified