[PATCH 1/6] sexp: fix invalid deallocation in error path
peter at lekensteyn.nl
Sat Jul 11 01:02:23 CEST 2015
On Fri, Jul 10, 2015 at 11:11:08AM +0900, NIIBE Yutaka wrote:
> Thank you for the fix. I'd like to commit your fix without your
> comment since it is not accurate. Is it OK for you?
You are right, I swapped the case. Feel free to adjust it to whatever
you deem appropriate.
> On 07/10/2015 12:11 AM, Peter Wu wrote:
> > --
> > This appears to be a copy and paste error and could result in wrong
> > memory being freed in the error path (when arrayisdesc[idx] == 2, the
> > condition ((!2) == 1) is false).
> When arrayisdesc[idx] == 1, it means that the buffer is provided by
> When arrayisdesc[idx] == 2, it means that the buffer is allocated here
> in _gcry_sexp_vextract_param.
> When arrayisdesc[idx] == 1, the wrong expression '!arrayisdesc[idx] == 1'
> is evaluated to !1 == 1 -> 0 == 1 -> 0 (false) and it doesn't go into
> the statements to set spec->len = 0 but goes into 'else' clause to
> free the buffer. THIS IS A PROBLEM.
Right, thanks for laying this down.
> When arrayisdesc[idx] == 2, the wrong expression '!arrayisdesc[idx] == 1'
> is evaluated to !2 == 1 -> 0 == 1 -> 0 (false) and it goes into 'else'
> clause (which is correct behaviour).
By accident, yes. This patch corrects the 1 case.
More information about the Gcrypt-devel