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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Gnupg-devel mailing list [email protected] https://lists.gnupg.org/mailman/listinfo/gnupg-devel
