[PATCH 1/6] sexp: fix invalid deallocation in error path

NIIBE Yutaka gniibe at fsij.org
Fri Jul 10 04:11:08 CEST 2015


Hello,

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?

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

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.

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



More information about the Gcrypt-devel mailing list