tz1 / qrduino

qr codes (2d barcodes) for arduino and embedded
GNU General Public License v3.0
70 stars 32 forks source link

Reading from uninitialized memory #1

Closed benkasminbullock closed 9 years ago

benkasminbullock commented 10 years ago

https://github.com/tz1/qrduino/blob/master/qrencode.c#L279

Sometimes x becomes 255 and this reads from uninitialized memory.

I put a check line and break; if x is out of bounds.

I found this with valgrind.

tz1 commented 10 years ago

What are the other conditions when this happens? Particularly WD and y. There is no read at that line - only in the ismasked() routine. Maybe the problem is with compiler optimization inlinine too agressively. Is this AVR or linux/unix?

benkasminbullock commented 10 years ago

I'm using FreeBSD. I run the program as follows:

 valgrind ./qrjpeg "butterfingers"

The valgrind output looks like this:

==41335== Memcheck, a memory error detector
==41335== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==41335== Using Valgrind-3.8.0 and LibVEX; rerun with -h for copyright info
==41335== Command: ./qrjpeg butterfingers
==41335== 
==41335== Invalid read of size 1
==41335==    at 0x804C7B4: qrencode (qrencode.c:279)
==41335==    by 0x8048841: main (qrjpeg.c:81)
==41335==  Address 0x17c722 is not stack'd, malloc'd or (recently) free'd
==41335== 
==41335== 
==41335== HEAP SUMMARY:
==41335==     in use at exit: 5,746 bytes in 6 blocks
==41335==   total heap usage: 6 allocs, 0 frees, 5,746 bytes allocated
==41335== 
==41335== LEAK SUMMARY:
==41335==    definitely lost: 0 bytes in 0 blocks
==41335==    indirectly lost: 0 bytes in 0 blocks
==41335==      possibly lost: 0 bytes in 0 blocks
==41335==    still reachable: 5,746 bytes in 6 blocks
==41335==         suppressed: 0 bytes in 0 blocks
==41335== Rerun with --leak-check=full to see details of leaked memory
==41335== 
==41335== For counts of detected and suppressed errors, rerun with: -v
==41335== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

When I added an assert at the line before the one in question, it said that x had a value of 255 sometimes. Add #include <stdio.h> at the top then

        if (x > WD - 1) {
            fprintf (stderr, "x = %d\n", x);
        }
        if (y > WD - 1) {
            fprintf (stderr, "y = %d\n", x);
        }
            } while (ismasked(x, y));

The result with the same above "butterfingers" input is that x = 255 is printed to stderr.

tz1 commented 9 years ago

Fixed. Rootcause was after the last actual data bit was inserted into the image, it still looked for the next open bit which didn't exist so would overflow the bottom left. Write/seek was changed to If(!zeroth) seek / write.