Fwd: New Defects reported by Coverity Scan for Das U-Boot

[Heinrich Schuchardt]

On 26.05.20 20:40, Tom Rini wrote:
> Ah, I thought you might not have been part of Coverity.
> https://scan.coverity.com/projects/das-u-boot is where to start, it will
> take GitHub credentials and then I can approve you.

Thanks for granting access. In the GUI one can really drill down into
the explanation of the problem. This is very helpful.

> ** CID 303760:    (TAINTED_SCALAR)
>
>
>
________________________________________________________________________________________________________
> *** CID 303760:    (TAINTED_SCALAR)
> /cmd/efidebug.c: 938 in show_efi_boot_order()
> 932                     }
> 933                     p = label;
> 934                     utf16_utf8_strncpy(&p, lo.label, label_len16);
> 935                     printf("%2d: %s: %s\n", i + 1, var_name, label);
> 936                     free(label);
> 937
>>>>     CID 303760:    (TAINTED_SCALAR)
>>>>     Passing tainted variable "data" to a tainted sink.
> 938                     free(data);
> 939             }
> 940     out:
> 941             free(bootorder);
> 942
> 943             return ret;
> /cmd/efidebug.c: 929 in show_efi_boot_order()
> 923                     efi_deserialize_load_option(&lo, data);
> 924
> 925                     label_len16 = u16_strlen(lo.label);
> 926                     label_len = utf16_utf8_strnlen(lo.label,
label_len16);
> 927                     label = malloc(label_len + 1);
> 928                     if (!label) {
>>>>     CID 303760:    (TAINTED_SCALAR)
>>>>     Passing tainted variable "data" to a tainted sink.
> 929                             free(data);
> 930                             ret = CMD_RET_FAILURE;
> 931                             goto out;
> 932                     }
> 933                     p = label;
> 934                     utf16_utf8_strncpy(&p, lo.label, label_len16);

In CID 303760 the logic is as follows:

In show_efi_boot_order() we malloc() memory. The allocated buffer is
filled via byte swapping (get_unaligned_le16(), get_unaligned_le32()).

Here comes Coverity's logic: "byte_swapping: Performing a byte swapping
operation on ... implies that it came from an external source, and is
therefore tainted."

Now we pass the pointer to free(). Free() looks at 16 bytes preceding
the pointer. Therefore free() is considered a tainted sink and an issue
is raised.

https://community.synopsys.com/s/article/From-Case-Clearing-TAINTED-STRING
suggests to use Coverity specific comments to mark cleansing functions.
This is not what I am inclined to do.

CCing Takahiro as he had a hand in this code.

Best regards

Heinrich