[scan-admin at coverity.com: New Defects reported by Coverity Scan for Das U-Boot]
Tom Rini
trini at konsulko.com
Mon Aug 16 22:20:27 CEST 2021
On Mon, Aug 16, 2021 at 10:15:49PM +0200, Pali Rohár wrote:
> + Stefan and Marek
>
> On Monday 16 August 2021 15:57:26 Tom Rini wrote:
> > Hey all,
> >
> > Can people please take a look? I can mark as intentional anything that
> > really is intentional, thanks.
>
> Hello Tom!
>
> These kwbimage issues look to be a real issues. But I do not think that
> anybody touched these parts of kwbimage code recently. So looks like
> that Coverity must have run some more tests this time...
Yeah, that happens from time to time.
>
> > ** CID 338491: Null pointer dereferences (NULL_RETURNS)
> > /tools/kwbimage.c: 1066 in export_pub_kak_hash()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 338491: Null pointer dereferences (NULL_RETURNS)
> > /tools/kwbimage.c: 1066 in export_pub_kak_hash()
> > 1060 int res;
> > 1061
> > 1062 hashf = fopen("pub_kak_hash.txt", "w");
> > 1063
> > 1064 res = kwb_export_pubkey(kak, &secure_hdr->kak, hashf, "KAK");
> > 1065
> > >>> CID 338491: Null pointer dereferences (NULL_RETURNS)
> > >>> Dereferencing a pointer that might be "NULL" "hashf" when calling "fclose".
> > 1066 fclose(hashf);
> > 1067
> > 1068 return res < 0 ? 1 : 0;
> > 1069 }
> > 1070
> > 1071 int kwb_sign_csk_with_kak(struct image_tool_params *params,
>
> There is really missing check that fopen() succeeded.
>
> > ** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
> > /tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
> > /tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak()
> > 1087 if (export_pub_kak_hash(kak, secure_hdr))
> > 1088 return 1;
> > 1089
> > 1090 if (kwb_import_pubkey(&kak_pub, &secure_hdr->kak, "KAK") < 0)
> > 1091 return 1;
> > 1092
> > >>> CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
> > >>> Using variable "csk_idx" as an index to array "secure_hdr->csk".
> > 1093 if (kwb_export_pubkey(csk, &secure_hdr->csk[csk_idx], NULL, "CSK") < 0)
> > 1094 return 1;
> > 1095
> > 1096 if (kwb_sign_and_verify(kak, &secure_hdr->csk,
> > 1097 sizeof(secure_hdr->csk) +
> > 1098 sizeof(secure_hdr->csksig),
>
> There is code:
>
> int csk_idx = image_get_csk_index();
> ...
> if (csk_idx >= 16) {
> ...
> return 1;
> }
> ... &secure_hdr->csk[csk_idx] ...
>
> And ->csk is defined as:
>
> struct secure_hdr_v1 {
> ..
> struct pubkey_der_v1 csk[16]
> ..
> };
>
> image_get_csk_index() returns int and it may returns also negative value
> on error. So there is really possible illegal memory access.
>
> > ** CID 338486: Null pointer dereferences (NULL_RETURNS)
> > /tools/kwbimage.c: 836 in kwb_dump_fuse_cmds()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 338486: Null pointer dereferences (NULL_RETURNS)
> > /tools/kwbimage.c: 836 in kwb_dump_fuse_cmds()
> > 830 return 0;
> > 831
> > 832 if (!strcmp(e->name, "a38x")) {
> > 833 FILE *out = fopen("kwb_fuses_a38x.txt", "w+");
> > 834
> > 835 kwb_dump_fuse_cmds_38x(out, sec_hdr);
> > >>> CID 338486: Null pointer dereferences (NULL_RETURNS)
> > >>> Dereferencing a pointer that might be "NULL" "out" when calling "fclose".
> > 836 fclose(out);
> > 837 goto done;
> > 838 }
> > 839
> > 840 ret = -ENOSYS;
> > 841
>
> And there is also missing check that fopen() succeeded.
Since you've been in here and analyzed things (thanks!) can you make a
few patches for things?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210816/7735d0b7/attachment.sig>
More information about the U-Boot
mailing list