Re: libtasn1 with a fine-toothed comb

[Pascal Cuoq]

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;
    }
  …

This needs to be fixed in any case regardless of what we do with the case len=0:

  unsigned char *new_value = realloc (node->value, node->value_len);
  if (new_value == NULL)
    {
      free(node->value);
      node->value = NULL;
      node->value_len = 0;
      return NULL;
    }
  node->value = new_value;
  …

With the above change and restructuring the function a bit, I arrive to the 
following commit:
https://github.com/pascal-cuoq/libtasn1-fork/commit/5af173a1528fbb7095549668158fcc60552a66ba

>> 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).

The production of new testcases, once the above 1500 or so are already 
available, is very slow, and would only make sense when the library's code had 
changed significantly or if the human in the loop had had a new creative idea 
for using afl. In this case afl would take advantage of the existing testcases 
to discover which paths are already covered in the new source code and which 
new edges in the Control Flow Graph it should attempt to generate testcases for.

In short, I would recommend you keep the testcases (and perhaps I can convince 
you to run tis-interpreter on a selection of them), but running afl more at 
this point would only add value if you had ideas for guiding it towards 
different parts of the code. Running afl again should be done every 6 months or 
every time you get an idea for generating significantly different testcases.


Pascal