Page MenuHome GnuPG
Create Task
Maniphest T4495

UBsan finding "certdump.c:695:3: runtime error: null pointer passed as argument 2"
Closed, ResolvedPublic
Actions

Description

Hi Everyone,

I'm performing a UBsan run on GnuPG 2.2.15 and all of its dependencies. Everything was built with -fsanitize=undefined -fno-sanitize-recover. For Autotools projects the two flags get placed in CFLAGS, CXXFLAGS and LDFLAGS.

It looks like UBsan is producing some findings:

Checking that a valid signature is verified as such.
("/home/build/gnupg-2.2.15/sm/gpgsm" --yes --verify "/tmp/gpgscm-20190510T190415-verify-fyLDre/sig" "/tmp/gpgscm-20190510T190415-verify-2dLyKY/body") failed: gpgsm: WARNING: running with faked system time: 2001-12-13 11:00:00
gpgsm: Signature made [date not given] using certificate ID 0x33BDB76E
gpgsm: CRLs not checked due to --disable-crl-checks option
certdump.c:695:3: runtime error: null pointer passed as argument 2, which is declared to never be null

0: tests.scm:122: (throw (string-append (stringify what) " failed") (:stderr result))
1: verify.scm:49: (call-check `(,@gpgsm --verify ,sig ,body))
FAIL: tests/gpgsm/verify.scm

Because -fno-sanitize-recover is used, abort() is called on a finding. It is needed because test frameworks often swallow the message *runtime error: ...* and the tester never gets to see it.

Revisions and Commits

rG GnuPG
rGc7ff8c59b925 sm: Fix a warning in an es_fopencooie function.
rG8d0d61aca3d2 sm: Fix a warning in an es_fopencooie function.

Related Objects

Event Timeline

JW created this task.May 10 2019, 9:24 PM
JW created this object in space S1 Public.
JW added a comment.EditedMay 10 2019, 9:26 PM

It looks like this patch clears this finding:

--- sm/certdump.c
+++ sm/certdump.c
@@ -692,9 +692,13 @@
       gpg_err_set_errno (c->error);
       return -1;
     }
-  memcpy (p + c->len, buffer, size);
-  c->len += size;
-  p[c->len] = 0; /* Terminate string. */
+
+  if (p + c->len && buffer)
+    {
+      memcpy (p + c->len, buffer, size);
+      c->len += size;
+      p[c->len] = 0; /* Terminate string. */
+    }

   return (gpgrt_ssize_t)size;
 }

That's it. Everything else in GnuPG tested good.

Two findings are very good results for UBsan. Good job.

• werner triaged this task as Normal priority.May 12 2019, 8:43 PM
• werner added a subscriber: • werner.

Thanks for the tests. I just fixed this one and will do replace some code in master, soon.

• werner added a commit: rG8d0d61aca3d2: sm: Fix a warning in an es_fopencooie function..May 12 2019, 8:43 PM
• gniibe mentioned this in T4899: Undefined behavior in sm/certdump.c.Apr 1 2020, 8:16 AM
• werner added a commit: rGc7ff8c59b925: sm: Fix a warning in an es_fopencooie function..Apr 1 2020, 9:59 AM
• werner closed this task as Resolved.Apr 1 2020, 10:00 AM
• werner claimed this task.

Applied the fix also to master with a comment to ebentually replace it with es_fopenmem.