Memory leaks

[rwhitworth]

Line 77 of optimize.c seems to have an off by one error, but I'm unable to determine how exactly. I think it is in instructions->instruction[i + (*position)], as clearLoop is already protected by the for loop iterator. Perhaps a check on instructions->size >= 3 should be done prior to the for loop?

Here is the code in question:

bool optimize_clear_loop(INSTRUCTIONS *instructions, int *position, CODE *code) {
        if (globalOptions.optimize < 2) // Only do on optimization level 2 and higher
                return false;

        char clearLoop[] = "[-]";
        int  i;
        for (i = 0; i < sizeof(clearLoop) - 1; i++) {
                if (instructions->instruction[i + (*position)].type != clearLoop[i])
                        return false;
        }

        construct_CLEAR(code);
        (*position) += sizeof(clearLoop) - 2;
        total_CLEARLOOP += 1;

        return true;
}

And this is the valgrind output that leads me to believe there is an error

==30266== Memcheck, a memory error detector
==30266== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30266== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==30266== Command: ./yabfc -s /tmp/input.3
==30266==
==30266== Invalid read of size 1
==30266==    at 0x4043AA: optimize_clear_loop (optimize.c:77)
==30266==    by 0x401716: main (yabfc.c:135)
==30266==  Address 0x56df9d0 is 0 bytes after a block of size 16 alloc'd
==30266==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==30266==    by 0x4015A6: main (yabfc.c:117)

[julianneswinoga]

Thank you very much for looking into this, I've not had the change to learn how to use valgrind much, but I think I'll look into it now. Here's what I'm getting:

==28879== 
==28879== HEAP SUMMARY:
==28879==     in use at exit: 18,728 bytes in 6 blocks
==28879==   total heap usage: 1,534 allocs, 1,528 frees, 5,193,299 bytes allocated
==28879== 
==28879== 0 bytes in 1 blocks are definitely lost in loss record 1 of 6
==28879==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x40236F: main (yabfc.c:90)
==28879== 
==28879== 7 bytes in 1 blocks are definitely lost in loss record 2 of 6
==28879==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x401743: filenameWithoutExtension (helpers.c:43)
==28879==    by 0x4022E0: main (yabfc.c:80)
==28879== 
==28879== 23 bytes in 1 blocks are definitely lost in loss record 3 of 6
==28879==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x4017C4: addSectionData (helpers.c:58)
==28879==    by 0x40296F: main (yabfc.c:199)
==28879== 
==28879== 5,365 bytes in 1 blocks are definitely lost in loss record 4 of 6
==28879==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x400B15: construct_arbitrary (assembly.c:10)
==28879==    by 0x4011DD: construct_END (assembly.c:161)
==28879==    by 0x402833: main (yabfc.c:179)
==28879== 
==28879== 5,365 bytes in 1 blocks are definitely lost in loss record 5 of 6
==28879==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x4017C4: addSectionData (helpers.c:58)
==28879==    by 0x402854: main (yabfc.c:181)
==28879== 
==28879== 7,968 bytes in 1 blocks are definitely lost in loss record 6 of 6
==28879==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28879==    by 0x402491: main (yabfc.c:117)
==28879== 
==28879== LEAK SUMMARY:
==28879==    definitely lost: 18,728 bytes in 6 blocks
==28879==    indirectly lost: 0 bytes in 0 blocks
==28879==      possibly lost: 0 bytes in 0 blocks
==28879==    still reachable: 0 bytes in 0 blocks
==28879==         suppressed: 0 bytes in 0 blocks
==28879== 
==28879== For counts of detected and suppressed errors, rerun with: -v
==28879== ERROR SUMMARY: 6 errors from 6 contexts (suppressed: 0 from 0)

18, 728 bytes lost, uh oh! 😮

[julianneswinoga]

Fixed

[rwhitworth]

@cameronswinoga I am still seeing the error present. With a given input file the instructions->size variable can be set to 1 which causes the invalid read.

input.1.gz

[julianneswinoga]

@rwhitworth I'm not sure what that file you attached was supposed to be, but I'm seeing it as garbled on my end.

00000000  5b cb e1 ff 2e                                    |[....|
00000005

[rwhitworth]

The file was generated by American Fuzzy Lop (afl-fuzz) and it is binary, you're right. You may have more luck understanding the problem by checking #8. Looks like a logic error that is fairly easy to protect against, but I'm not certain if this is the best way to handle it.

May be worth enforcing ascii input to the program by using isdigit or isalpha, or whatever method makes the most sense.

[julianneswinoga]

ASCII input is already done by only adding the input to the command structure if the character is one of "+-<>[],.". This is done on line 112 of yabfc.c.