Home GnuPG
Diffusion GPGME 7c220e387d51

core: Initialize key return value in gpgme_get_key
7c220e387d51
Actions

Description

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.

Details

Provenance
• aheineckeAuthored on Mar 27 2018, 11:24 AM
Parents
rM40a9dea5d565: script: temp homedir
Branches
Unknown
Tags
Unknown
Tasks
T3865: Kleopatra crashes again in Gpg4win 3.1.0 beta 38

Event Timeline

• aheinecke added a task: T3865: Kleopatra crashes again in Gpg4win 3.1.0 beta 38.Mar 27 2018, 11:29 AM
• aheinecke committed rM7c220e387d51: core: Initialize key return value in gpgme_get_key (authored by • aheinecke).Mar 27 2018, 11:24 AM
• werner added a subscriber: • werner.Mar 28 2018, 9:21 PM

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.

• aheinecke added a comment.Mar 29 2018, 8:54 AM

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.

Changes (1)

PathSize
src/

rM7c220e387d51

View Options

src/keylist.c

Loading...