Created attachment 263110 [details] [review]
gcr-trust: Fix a potential NULL pointer dereference

If error is NULL, this will crash.

Found by scan-build.

This function's error handling doesn't look right to me, wit or without the patch. perform_add_pinned_certificate() is called from a public function, gcr_trust_add_pinned_certificate() and thus error might be NULL.  

GError *lerr = NULL;
[...]
       /* We need an error below */
        if (error && !*error)
                *error = lerr;
[...]
        object = gck_enumerator_next (en, cancellable, error);
[...]
        if (*error)
                return FALSE;
(or with your patch, if (error && *error))

However, if error == NULL, then we still need to return here if gck_enumator_next() failed. 

So really this function should use lerr, and propagate to @error on failures.

Created attachment 263296 [details] [review]
gcr-trust: Fix a potential NULL pointer dereference

The error handling in perform_add_pinned_certificate() didn’t allow for
error to be NULL, but it could easily have been NULL since
perform_add_pinned_certificate() is called from public functions with
GError arguments.

Rework the error handling to use a local GError and propagate it to the
caller. This should prevent crashes if error is NULL.

Found by scan-build.

Thanks.

Attachment 263296 [details] pushed as decba1e - gcr-trust: Fix a potential NULL pointer dereference