From libosmo-abis beb10ef02a10d73537a97f6f21aad36664c9b266 "add basic unixsocket support":
On Wed, Mar 08, 2017 at 08:39:07AM -0800, scan-admin@coverity.com wrote:
*** CID 163920: (TAINTED_SCALAR) /source-Osmocom/libosmo-abis/src/input/unixsocket.c: 90 in unixsocket_read_cb() 84 uint8_t controldata; 85 int ret; 86 87 if (!msg) 88 return -ENOMEM; 89
CID 163920: (TAINTED_SCALAR) Calling function "read" taints argument "msg->data".90 ret = read(bfd->fd, msg->data, UNIXSOCKET_ALLOC_SIZE - 16);
What this seems to complain about is that we're writing arbitrary data to msg->data, and in lapd_receive() pass this to a LOGP that prints a hexdump of it. In osmo_hexdump, we of course do hex_chars[buf[i] >> 4] which we know to always work out to one of '0'..'f' for *any* data. This point seems to be missed by covertiy scan and it believes we may end up with an invalid index to hex_chars[]. So this is a false positive. Marked it as such.
*** CID 163919: Security best practices violations (STRING_OVERFLOW) /source-Osmocom/libosmo-abis/src/input/unixsocket.c: 236 in unixsocket_line_update() 230 struct unixsocket_line *config; 231 char sock_path[PATH_MAX]; 232 int ret = 0; 233 int i; 234 235 if (line->sock_path)
CID 163919: Security best practices violations (STRING_OVERFLOW) Note: This defect has an elevated risk because the source argument is a parameter of the current function.236 strcpy(sock_path, line->sock_path);
Please always use osmo_strlcpy(), here with sizeof(sock_path).
237 else 238 sprintf(sock_path, "%s%d", UNIXSOCKET_SOCK_PATH_DEFAULT, 239 line->num); 240 241 LOGP(DLINP, LOGL_NOTICE, "line update (line=%p)\n", line);
** CID 163918: Memory - illegal accesses (BUFFER_SIZE_WARNING) /source-Osmocom/openbsc/openbsc/src/libbsc/bsc_subscriber.c: 83 in bsc_subscr_set_imsi()
The next one is from me, in 6d804b1a7e375213cb4b3e437c2b9b8c68872164 "add struct bsc_subscr, separating libbsc from gsm_subscriber":
*** CID 163918: Memory - illegal accesses (BUFFER_SIZE_WARNING) /source-Osmocom/openbsc/openbsc/src/libbsc/bsc_subscriber.c: 83 in bsc_subscr_set_imsi() 77 } 78 79 void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi) 80 { 81 if (!bsub) 82 return;
CID 163918: Memory - illegal accesses (BUFFER_SIZE_WARNING) Calling strncpy with a maximum size argument of 16 bytes on destination array "bsub->imsi" of size 16 bytes might leave the destination string unterminated.83 strncpy(bsub->imsi, imsi, sizeof(bsub->imsi));
Again, I should use osmo_strlcpy(), which correctly handles sizeof(buf), other than strncpy which might leave the string unterminated.
~N
-
Neels Hofmeyr