Re: [Xen-devel] [PATCH 04/14] fuzz/x86_emulate: Add a better input size check

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On Fri, Sep 15, 2017 at 12:39 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Aug 25, 2017 at 05:43:33PM +0100, George Dunlap wrote:
>> For some reason the 'feof()' check for the file size isn't working in
>> llvm-clang-fast mode; the result is several kilobyte files rather than
>> the 4k limit files as we've requested.  This is bad in part because
>> AFL will spend time trying to "fuzz" bits of the input that are never
>> touched.
>>
>
> You mean feof returns non-zero (true) when it shouldn't?

It looks like it does.  I modified the code thus:

        if ( !feof(fp) )
        {
            printf("Input too large\n");
            if ( optind + 1 ==  argc )
                exit(-1);
            continue;
        }

        if ( fread(input, 1, 1, fp) > 0 )
        {
            fprintf(stderr, "feof check failed to detect oversized input!");
            abort();
        }

And ran AFL for a bit in afl-clang-fast mode.  It ran fine for about
two cycles, before the massive repetition started happening; but once
the file size got larger than 4096 it found "crashes", even though
running it manually with the same input file results in a simple
"Input too large".

>> Add a new check: Offer to read INPUT_SIZE + 1; if we actually get that
>> many bytes, return an error.
>>
>> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
>> ---
>> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>>  tools/fuzz/x86_instruction_emulator/afl-harness.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/fuzz/x86_instruction_emulator/afl-harness.c 
>> b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> index 1a79ff228e..51e0183356 100644
>> --- a/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> +++ b/tools/fuzz/x86_instruction_emulator/afl-harness.c
>> @@ -76,7 +76,7 @@ int main(int argc, char **argv)
>>              }
>>          }
>>
>> -        size = fread(input, 1, INPUT_SIZE, fp);
>> +        size = fread(input, 1, INPUT_SIZE + 1, fp);
>
> You probably want to actual define input to be of INPUT_SIZE+1 byte as well.
>
> I doubt address sanitiser will be happy with overrunning the buffer.

Yeah, the check is a little suboptimal; I'll see what I can come up with.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel