From libosmo-abis beb10ef02a10d73537a97f6f21aad36664c9b266 "add basic unixsocket support":
On Wed, Mar 08, 2017 at 08:39:07AM -0800, [email protected] 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
signature.asc
Description: Digital signature
