On Mon, Apr 4, 2016 at 10:40 AM, Pascal Cuoq <[email protected]> wrote: > Hello, > On 03 Apr 2016, at 10:05, Nikos Mavrogiannopoulos > <[email protected]> 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
