cesanta / mongoose

Embedded Web Server
https://mongoose.ws
Other
10.93k stars 2.7k forks source link

New Defects reported by Coverity Scan for 7.14 #2834

Closed linkjumper closed 1 month ago

linkjumper commented 1 month ago

Hi,

When updating to the latest mongoose version 7.14, Coverity Scan detects the following defects.

** CID 500725: Control flow issues (UNREACHABLE)
/mongoose/mongoose.c: 15724 in mg_phy_id_to_str()

________________________________________________________________________________________________________
*** CID 500725: Control flow issues (UNREACHABLE)
/mongoose/mongoose.c: 15724 in mg_phy_id_to_str()
15718 return "LAN87x";
15719 case MG_PHY_RTL8201:
15720 return "RTL8201";
15721 default:
15722 return "unknown";
15723 }
>>> CID 500725: Control flow issues (UNREACHABLE)
>>> This code cannot be reached: "(void)id2;".
15724 (void) id2;
15725 }
15726
15727 void mg_phy_init(struct mg_phy *phy, uint8_t phy_addr, uint8_t config) {
15728 uint16_t id1, id2;
15729 phy->write_reg(phy_addr, MG_PHY_REG_BCR, MG_BIT(15)); // Reset PHY
** CID 500723: Integer handling issues (INTEGER_OVERFLOW)
/mongoose/mongoose.c: 2829 in mg_http_serve_file()

________________________________________________________________________________________________________
*** CID 500723: Integer handling issues (INTEGER_OVERFLOW)
/mongoose/mongoose.c: 2829 in mg_http_serve_file()
2823
2824 // Handle Range header
2825 struct mg_str *rh = mg_http_get_header(hm, "Range");
2826 range[0] = '\0';
2827 if (rh != NULL && (n = getrange(rh, &r1, &r2)) > 0) {
2828 // If range is specified like "400-", set second limit to content len
>>> CID 500723: Integer handling issues (INTEGER_OVERFLOW)
>>> Expression "cl - 1UL", which is equal to 18446744073709551615, where "cl" is known to be equal to 0, underflows the type that receives it, an unsigned integer 64 bits wide.
2829 if (n == 1) r2 = cl - 1;
2830 if (r1 > r2 || r2 >= cl) {
2831 status = 416;
2832 cl = 0;
2833 mg_snprintf(range, sizeof(range), "Content-Range: bytes */%lld\r\n",
2834 (int64_t) size);
** CID 500722: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 7874 in mg_wakeup()

________________________________________________________________________________________________________
*** CID 500722: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 7874 in mg_wakeup()
7868 bool mg_wakeup(struct mg_mgr *mgr, unsigned long conn_id, const void *buf,
7869 size_t len) {
7870 if (mgr->pipe != MG_INVALID_SOCKET && conn_id > 0) {
7871 char *extended_buf = (char *) alloca(len + sizeof(conn_id));
7872 memcpy(extended_buf, &conn_id, sizeof(conn_id));
7873 memcpy(extended_buf + sizeof(conn_id), buf, len);
>>> CID 500722: Error handling issues (CHECKED_RETURN)
>>> Calling "send(mgr->pipe, extended_buf, len + 8UL, 0)" without checking return value. This library function may fail and return an error code.
7874 send(mgr->pipe, extended_buf, len + sizeof(conn_id), MSG_NONBLOCKING);
7875 return true;
7876 }
7877 return false;
7878 }
7879
** CID 500721: Memory - illegal accesses (INCOMPATIBLE_CAST)

________________________________________________________________________________________________________
*** CID 500721: Memory - illegal accesses (INCOMPATIBLE_CAST)
/mongoose/mongoose.c: 4318 in mg_mqtt_next_prop()
4312 case MQTT_PROP_TYPE_BINARY_DATA:
4313 prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
4314 prop->val.buf = (char *) i + 2;
4315 new_pos += 2 + prop->val.len;
4316 break;
4317 case MQTT_PROP_TYPE_VARIABLE_INT:
>>> CID 500721: Memory - illegal accesses (INCOMPATIBLE_CAST)
>>> Pointer "&prop->iv" points to an object whose effective type is "unsigned int" (32 bits, unsigned) but is dereferenced as a wider "unsigned long" (64 bits, unsigned). This may lead to memory corruption.
4318 len = decode_varint(i, (size_t) (end - i), (size_t *) &prop->iv);
4319 new_pos = (!len) ? 0 : new_pos + len;
4320 break;
4321 default:
4322 new_pos = 0;
4323 }
** CID 500720: Integer handling issues (INTEGER_OVERFLOW)
/mongoose/mongoose.c: 2468 in mg_http_parse_headers()

________________________________________________________________________________________________________
*** CID 500720: Integer handling issues (INTEGER_OVERFLOW)
/mongoose/mongoose.c: 2468 in mg_http_parse_headers()
2462 if (k.len == 0) return false; // Empty name
2463 if (s >= end || clen(s, end) == 0) return false; // Invalid UTF-8
2464 if (*s++ != ':') return false; // Invalid, not followed by :
2465 // if (clen(s, end) == 0) return false; // Invalid UTF-8
2466 while (s < end && s[0] == ' ') s++; // Skip spaces
2467 if ((s = skiptorn(s, end, &v)) == NULL) return false;
>>> CID 500720: Integer handling issues (INTEGER_OVERFLOW)
>>> Expression "v.len--", which is equal to 18446744073709551615, where "v.len" is known to be equal to 0, underflows the type that receives it, an unsigned integer 64 bits wide.
2468 while (v.len > 0 && v.buf[v.len - 1] == ' ') v.len--; // Trim spaces
2469 // MG_INFO(("--HH [%.*s] [%.*s]", (int) k.len, k.buf, (int) v.len, v.buf));
2470 h[i].name = k, h[i].value = v; // Success. Assign values
2471 }
2472 return true;
2473 }

Best regards, Michael

scaprile commented 1 month ago

Do these actually affect you in any way ? Do you have an actual issue ? Please let me tell you that you are not the first one reporting false positives. If you are a paying customer and this is important to you, please contact Support by mail as per contract. Otherwise, you can search for past issues where we've analyzed all false positives. If you have an actual issue, please honor the issue template so we can know your scenario and reproduce the issue if necessary, and let us know exactly what you are doing and how this affects you. I'll take a look at these later anyway, please let me know if this issue needs to be re-opened. Thank you.

scaprile commented 1 month ago

First one: your coverity clearly doesn't know what it is checking. Second one: your coverity clearly doesn't understand the program flow and how that variable has been set by a function call. Third one: we are free to not check return values to reduce bloating, we know what we are doing. Fourth one: no, the function being called does know what it does. Fifth one: same as second one.