automated cppcheck for gnupg

[Leo Gaspard]

On Wed, Apr 16, 2014 at 09:21:20AM +0200, Werner Koch wrote:
> On Tue, 15 Apr 2014 23:35, ekleog at gmail.com said:
> > keeping it private costs virtually nothing. (The null dereference in libassuan
> > that Werner said he would fix tomorrow might be specially important,
> 
> No it isn't:  The code is similar to this
> 
>    ctx = malloc (sizeof *ctx);
>    if (!ctx)
>       {
>          trace_error (ctx->err_source)
>          return NULL;
>       }
> 
> but uses a macro for the tracing.  This is a simple NULL deref which
> raises a segv (because for ages OSes do not map the first memory page).
> I had to replace a macro with a inline function to avoid double
> evaluation of a macro parameter.

[I'm assuming I'm not that off-topic, since you answered.]

Well... Actually, not long ago, I read about a null pointer dereference leading
to complete system compromise (it was used on a root-run program). Sure, it was
used in conjunction with a mac os x flaw (which forced the first page to be
mapped read-only to some kernel-space memory, but did not give any kernel-level
access); yet unfortunately I can't remember where I found it, nor the exact
details... (Only that it was something that tried to explain almost any kind of
bug can be turned into a "remote" code injection... perhaps a book, now that I
think about it..)

It thus requires local access to the system (anyway, to trigger a malloc
failure...), yet would (IIRC) allow some other user (or daemon) to peep into
your running gpg-agent (as, AFAIK, gpg-agent uses libassuan), and so uncovering
your secret key.

Here, the argument to gpg_strsource (assuming the point of the mistake is the
one of the latest git commit on libassuan) would be attacker-controlled, as a
consequence the one to gpg_err_source. Doing no check, it means the argument to
msgidxof would be attacker-controlled. I unfortunately do not know enough about
gettext to know whether dgettext and/or msgidxof are secure enough to (i) not
overflow the msgidx buffer, and (ii) not trigger a vulnerability in dgettext
and/or msgidxof.

Anyway, that's fixed now!

Cheers,

Leo