Re: libtasn1 with a fine-toothed comb

[Nikos Mavrogiannopoulos]

On Mon, Apr 4, 2016 at 10:40 AM, Pascal Cuoq <address@hidden> wrote:
> Hello,
> On 03 Apr 2016, at 10:05, Nikos Mavrogiannopoulos <address@hidden> wrote:
>>>
>>> If it is not a logic error for _asn1_append_value() to be used this
>>> way, then ideally the function should handle the case where len is 0
>>> without calling memcpy on “one past the end” pointers to prevent
>>> future compiler optimizations to mess with that code.
>>
>> Would you like to suggest a patch for this fix?
>
> Ok. In any case, since we are looking at it, this use of realloc() in 
> _asn1_append_value() has a memory leak. The old memory block continues to 
> exist when realloc() returns NULL:
>
>   node->value = realloc (node->value, node->value_len);
>   if (node->value == NULL)
>     {
>       node->value_len = 0;
>       return NULL;
>     }
>   …

Thank you. I've addressed that a bit differently, but I liked your
cleanup and I've adopted it. For the realloc issue I've defined a
safer variant of realloc that handles the case of failed allocation.
I've ignored asn1Decoding.c as it is a one-time off application and
catching memory leaks on memory allocation doesn't make much sense.

>>> PS: I have used afl to generate inputs that exert (so far) 1206
>>> different execution paths inside asn1Decode. I will make all the
>>> inputs available in batch at the end of the fuzzing session, but I
>>> would appreciate additional testcases, especially if they came in the
>>> form of inputs to asn1Decode.
>>
>> I'm wondering, could afl be automated and used as part of the libtasn1
>> testsuite, or some extended test suite?
> The generated testcases have immense value: that's 1480 inputs (by now after 
> three days of fuzzing) that cover a lot of corner cases of the code, in a way 
> that's very complementary of human-produced testcases. The whole suite runs 
> in minutes with cheap-and-efficient instrumentation like ASan or Valgrind 
> (and quite a bit longer with tis-interprete because tis-interpreter looks for 
> more sorts of problems).

I don't know how easy would that be, but would you like to submit them
in a patch?

regards,
Nikos