Home GnuPG
Diffusion GPGME 7c220e387d51

core: Initialize key return value in gpgme_get_key


core: Initialize key return value in gpgme_get_key

* src/keylist.c (gpgme_get_key): Set r_key to NULL.

The c++ bindings and others assumed that r_key is set to NULL
on error. This is the behavior gpgme_op_keylist_next also
has. Even if it is not specified what happens to r_key on
error setting it to NULL should not hurt and is more
expected behavior.

This directly fixes an uninitialized memory access error
in the c++ bindings / Kleopatra:

And will fix some additional random crashes in Kleopatra and GpgOL.


aheineckeAuthored on Mar 27 2018, 11:24 AM
rM40a9dea5d565: script: temp homedir
T3865: Kleopatra crashes again in Gpg4win 3.1.0 beta 38

Event Timeline

I also stumbled on this problem in the past. However, I did not changed it because I feared to break callers which expect that the passed key object is not changed on error. This may happen in a loop where you test for an ambiguous key and don't save zway the first retrieved key. In the error case (i.e. not ambiguous) this would dump the good key object.

I am not sure whether this is still a real problem, but we should be aware of this. You are right the behaviour is/was not specified and thus a caller should not rely on a non-changing key object in the error case.

I might have changed the caller (GpgME++) but what convinced me that the change in core is better is that op_keylist_next sets the return value to NULL on error.

I find it very difficult to imagine a way to rely on the behavior that the pointer is not changed on error without leaking memory on success. I mean a caller would have to keep a copy of the pointer for that case anyway.