Re: [Xen-devel] [PATCH] fuzz/x86_emulate: fix bounds for input size

[Paul Semel <semelpaul@xxxxxxxxx>]

On 02/23/2018 12:57 AM, Paul Semel wrote:
> The minimum size for the input size was set to DATA_OFFSET + 1 which was meaning
> that we were requesting at least one character of the data array to be filled.
> This is not needed for the fuzzer to get working correctly.
>
> The maximum size for the input size was set to INPUT_SIZE, which is actually
> the size of the data array inside the fuzz_corpus structure and so was not
> abling user (or AFL) to fill in the whole structure. Changing to
> sizeof(struct fuzz_corpus) correct this problem.
>
> Signed-off-by: Paul Semel <semelpaul@xxxxxxxxx>
> ---
>   tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
> b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> index 964682aa1a..f3ce2e7e27 100644
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -33,6 +33,7 @@ struct fuzz_corpus
>       unsigned char data[INPUT_SIZE];
>   } input;
>   #define DATA_OFFSET offsetof(struct fuzz_corpus, data)
> +#define FUZZ_CORPUS_SIZE (sizeof(struct fuzz_corpus))
>   
>   /*
>    * Internal state of the fuzzing harness.  Calculated initially from the input
> @@ -822,13 +823,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t 
> size)
>       /* Reset all global state variables */
>       memset(&input, 0, sizeof(input));
>   
> -    if ( size <= DATA_OFFSET )
> +    if ( size < DATA_OFFSET )
>       {
>           printf("Input too small\n");
>           return 1;
>       }
>   
> -    if ( size > INPUT_SIZE )
> +    if ( size > FUZZ_CORPUS_SIZE )
>       {
>           printf("Input too large\n");
>           return 1;
> @@ -859,9 +860,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t 
> size)
>   
>   unsigned int fuzz_minimal_input_size(void)
>   {
> -    BUILD_BUG_ON(DATA_OFFSET > INPUT_SIZE);
> +    BUILD_BUG_ON(DATA_OFFSET > FUZZ_CORPUS_SIZE);
Actually, this check is dumb.

I think I should rather do :
        BUILD_BUG_ON(DATA_OFFSET + INPUT_SIZE != FUZZ_CORPUS_SIZE)

This way, we ensure that the data field is the last field of fuzz_corpus 
structure.
What do you think about this ?

>   
> -    return DATA_OFFSET + 1;
> +    return DATA_OFFSET;
>   }
>   
>   /*
--
Paul Semel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel