gpgme static analysis findings
Ingo Klöcker
kloecker at kde.org
Mon Jul 15 22:50:28 CEST 2024
On Montag, 15. Juli 2024 18:17:59 CEST Michal Hlavinka via Gnupg-devel wrote:
> Hi,
>
> we've started to run static analysis on some system components including
> gpgme where the scanner reported a few issues:
Thanks.
> 1) in src/engine.c
> """
> "Error: OVERRUN (CWE-119):
> src/engine.c:121: cond_between: Checking ""proto > 7UL"" implies that
> ""proto"" is between 0 and 7 (inclusive) on the false branch.
> src/engine.c:125: overrun-local: Overrunning array ""engine_ops"" of 7
> 8-byte elements at element index 7 (byte offset 63) using index
> ""proto"" (which evaluates to 7).
> # 123|
> # 124| if (engine_ops[proto] && engine_ops[proto]->get_req_version)
> # 125|-> return (*engine_ops[proto]->get_req_version) ();
> # 126| else
> # 127| return NULL;"
> """
>
> 121: if (proto > DIM (engine_ops))
>
> this checks if 'proto' is bigger than number of elements in engine_ops
>
> DIM is defined in util.h:
> 44: #define DIM(v) (sizeof(v)/sizeof((v)[0]))
>
> the above condition allows for proto = DIM(engine_ops)
> which later at:
> 125: return (*engine_ops[proto]->get_req_version) ();
> allows to access (proto+1)th element
>
> seems that the condition should actually be proto >= DIM(engine_ops)
I agree. There's no real danger of an overrun because the enum enumerating the
protocols only lists 7 values (0-6) and the two additional enum values 254 and
255 which are much > DIM (engine_ops). Of course, this should still be fixed.
> 2) in src/gpgme-tool.c
> simple list assignment as used at gt_get_keylist_mode () does not
> prevent out of bound access.
>
> """
> Error: OVERRUN (CWE-119):
> src/gpgme-tool.c:1445: assignment: Assigning: ""idx"" = ""0"".
> src/gpgme-tool.c:1449: incr: Incrementing ""idx"". The value of ""idx""
> is now 1.
> src/gpgme-tool.c:1451: incr: Incrementing ""idx"". The value of ""idx""
> is now 2.
> src/gpgme-tool.c:1453: incr: Incrementing ""idx"". The value of ""idx""
> is now 3.
> src/gpgme-tool.c:1455: incr: Incrementing ""idx"". The value of ""idx""
> is now 4.
> src/gpgme-tool.c:1457: incr: Incrementing ""idx"". The value of ""idx""
> is now 5.
> src/gpgme-tool.c:1459: incr: Incrementing ""idx"". The value of ""idx""
> is now 6.
> src/gpgme-tool.c:1461: incr: Incrementing ""idx"". The value of ""idx""
> is now 7.
> src/gpgme-tool.c:1463: incr: Incrementing ""idx"". The value of ""idx""
> is now 8.
> src/gpgme-tool.c:1464: overrun-local: Overrunning array ""modes"" of 7
> 8-byte elements at element index 8 (byte offset 71) using index
> ""idx++"" (which evaluates to 8).
> # 1462| if (mode & GPGME_KEYLIST_MODE_FORCE_EXTERN)
> # 1463| modes[idx++] = ""force_extern"";
> # 1464|-> modes[idx++] = NULL;
> # 1465|
> # 1466| gt_write_status (gt, STATUS_KEYLIST_MODE, modes[0],
> modes[1], modes[2],
> """
This has been fixed. And while at it I have added support for some more keylist
modes. https://dev.gnupg.org/rMaa15a664b3cf9bf578ba6d22c1c0c327af68b1b4
Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20240715/10c2abdd/attachment.sig>
More information about the Gnupg-devel
mailing list