On 5/8/2023 3:20 PM, Tom Rini wrote:
Here's the latest defect report:
---------- Forwarded message --------- From:scan-admin@coverity.com Date: Mon, May 8, 2023, 2:29 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To:tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
5 new defect(s) introduced to Das U-Boot found with Coverity Scan. 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 5 of 5 defect(s)
** CID 450971: Insecure data handling (TAINTED_SCALAR) /net/ndisc.c: 391 in process_ra()
*** CID 450971: Insecure data handling (TAINTED_SCALAR) /net/ndisc.c: 391 in process_ra() 385 /* Ignore the packet if router lifetime is 0. */ 386 if (!icmp->icmp6_rt_lifetime) 387 return -EOPNOTSUPP; 388 389 /* Processing the options */ 390 option = msg->opt;
CID 450971: Insecure data handling (TAINTED_SCALAR) Using tainted variable "remaining_option_len" as a loop boundary.391 while (remaining_option_len > 0) { 392 /* The 2nd byte of the option is its length. */ 393 option_len = option[1]; 394 /* All included options should have a positive length. */ 395 if (option_len == 0) 396 return -EINVAL;
The problem here is that although the lower bound of the variable remaining_option_len is checked, the upper bound is not checked. Coverity is complaining that the function's argument len which is read from a packet content is assigned to remaining_option_len and therefore has made it a tainted scalar.
I will compare the value of len with ETH_MAX_MTU constant and make sure it is less than that as shown below.
if(len > ETH_MAX_MTU) return-EMSGSIZE;
** CID 450969: Security best practices violations (DC.WEAK_CRYPTO) /net/ndisc.c: 209 in ip6_send_rs()
*** CID 450969: Security best practices violations (DC.WEAK_CRYPTO) /net/ndisc.c: 209 in ip6_send_rs() 203 icmp_len, PROT_ICMPV6, pcsum); 204 msg->icmph.icmp6_cksum = csum; 205 pkt += icmp_len; 206 207 /* Wait up to 1 second if it is the first try to get the RA */ 208 if (retry_count == 0)
CID 450969: Security best practices violations (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications,because linear congruential algorithms are too easy to break. 209 udelay(((unsigned int)rand() % 1000000) * MAX_SOLICITATION_DELAY); 210 211 /* send it! */ 212 net_send_packet(net_tx_packet, (pkt - net_tx_packet)); 213 214 retry_count++;
This is a false positive. The function rand() is not used for encryption here. It is used to just make a random delay to avoid collisions on the network. It has nothing to do with encryption.