This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Neels Hofmeyr nhofmeyr at sysmocom.deOn Mon, Sep 19, 2016 at 06:02:45AM -0700, scan-admin at coverity.com wrote:
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Osmocom found with Coverity Scan.
>
> 5 new defect(s) introduced to Osmocom found with Coverity Scan.
> 6 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 148210: Error handling issues (NEGATIVE_RETURNS)
> /source-iuh/openbsc/openbsc/src/gprs/gprs_llc.c: 273 in rx_llc_xid()
>
>
> ________________________________________________________________________________________________________
> *** CID 148210: Error handling issues (NEGATIVE_RETURNS)
> /source-iuh/openbsc/openbsc/src/gprs/gprs_llc.c: 273 in rx_llc_xid()
> 267
> 268 response_len =
> 269 gprs_llc_process_xid_ind(gph->data, gph->data_len,
> 270 response, sizeof(response),
> 271 lle);
> 272 xid = msgb_put(resp, response_len);
> >>> CID 148210: Error handling issues (NEGATIVE_RETURNS)
> >>> "response_len" is passed to a parameter that cannot be negative.
@dexter ^
check that response_len > 0
> 273 memcpy(xid, response, response_len);
> 274
> 275 gprs_llc_tx_xid(lle, resp, 0);
> 276 } else {
> 277 LOGP(DLLC, LOGL_NOTICE,
> 278 "Received XID confirmation from phone.\n");
>
> ** CID 148209: Control flow issues (MISSING_BREAK)
> /source-iuh/openbsc/openbsc/src/libmsc/db.c: 414 in check_db_revision()
>
>
> ________________________________________________________________________________________________________
> *** CID 148209: Control flow issues (MISSING_BREAK)
> /source-iuh/openbsc/openbsc/src/libmsc/db.c: 414 in check_db_revision()
> 408
> 409 /* Incremental migration waterfall */
> 410 switch (db_rev) {
> 411 case 2:
> 412 if (update_db_revision_2())
> 413 goto error;
> >>> CID 148209: Control flow issues (MISSING_BREAK)
> >>> The above case falls through to this one.
add:
/* fall through */
?
> 414 case 3:
> 415 if (update_db_revision_3())
> 416 goto error;
> 417
> 418 /* The end of waterfall */
> 419 break;
>
> ** CID 148208: Control flow issues (DEADCODE)
> /source-iuh/openbsc/openbsc/src/osmo-bsc/osmo_bsc_ctrl.c: 408 in set_net_timezone()
>
>
> ________________________________________________________________________________________________________
> *** CID 148208: Control flow issues (DEADCODE)
> >>> CID 148208: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "0L" inside this statement: "tz->hr = (hourstr ? atol(ho...".
The website has more detail:
"
> static int set_net_timezone(struct ctrl_cmd *cmd, void *data)
> 386{
> 387 char *saveptr, *hourstr, *minstr, *dststr, *tmp = 0;
> 388 int override;
> 389 struct gsm_network *net = (struct gsm_network*)cmd->node;
> 390
> 391 tmp = talloc_strdup(cmd, cmd->value);
> 392 if (!tmp)
> 393 goto oom;
> 394
> 395 hourstr = strtok_r(tmp, ",", &saveptr);
> 396 minstr = strtok_r(NULL, ",", &saveptr);
> 397 dststr = strtok_r(NULL, ",", &saveptr);
> 398
> 399 override = 0;
> 400
cond_notnull: Condition hourstr != NULL, taking true branch. Now the value of hourstr is not NULL.
> 401 if (hourstr != NULL)
> 402 override = strcasecmp(hourstr, "off") != 0;
> 403
> 404 struct gsm_tz *tz = &net->tz;
> 405 tz->override = override;
> 406
> 407 if (override) {
notnull: At condition hourstr, the value of hourstr cannot be NULL.
dead_error_condition: The condition hourstr must be true.
CID 148208 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution cannot reach the expression 0L inside this statement: tz->hr = (hourstr ? atol(ho....
> 408 tz->hr = hourstr ? atol(hourstr) : 0;
> 409 tz->mn = minstr ? atol(minstr) : 0;
> 410 tz->dst = dststr ? atol(dststr) : 0;
> 411 }
> 412
> 413 talloc_free(tmp);
> 414 tmp = NULL;
> 415
> 416 return get_net_timezone(cmd, data);
> 417
> 418oom:
> 419 cmd->reply = "OOM";
> 420 return CTRL_CMD_ERROR;
> 421}
"
So override == 0 by default. override will only be set if hourstr != NULL.
So inside 'if (override)', hourstr will always be != NULL, and the first '?' is
superfluous. This exists since the function was added in 2013.
> ________________________________________________________________________________________________________
> *** CID 148207: Control flow issues (MISSING_BREAK)
> /source-iuh/openbsc/openbsc/src/libbsc/abis_rsl.c: 415 in channel_mode_from_lchan()
> 409 default:
> 410 LOGP(DRSL, LOGL_ERROR,
> 411 "unsupported lchan->csd_mode %u\n",
> 412 lchan->csd_mode);
> 413 return -EINVAL;
> 414 }
> >>> CID 148207: Control flow issues (MISSING_BREAK)
> >>> The above case falls through to this one.
This missing 'break' is an error introduced in
e4227988665cae6e44fc0f77c2463aae759f15ca
"RSL: Add basic support for CSD transparent mode"
> 415 default:
> 416 LOGP(DRSL, LOGL_ERROR,
> 417 "unsupported lchan->tch_mode %u\n",
> 418 lchan->tch_mode);
> 419 return -EINVAL;
> 420 }
>
> ** CID 148206: Security best practices violations (DC.WEAK_CRYPTO)
> /source-iuh/openbsc/openbsc/src/gprs/gprs_gmm.c: 560 in gsm48_tx_gmm_auth_ciph_req()
>
>
> ________________________________________________________________________________________________________
> *** CID 148206: Security best practices violations (DC.WEAK_CRYPTO)
> /source-iuh/openbsc/openbsc/src/gprs/gprs_gmm.c: 560 in gsm48_tx_gmm_auth_ciph_req()
> 554 /* 10.5.5.7: */
> 555 acreq->force_stby = force_standby;
> 556 /* 3GPP TS 24.008 10.5.5.19: */
> 557 if (RAND_bytes(&rbyte, 1) != 1) {
> 558 LOGP(DMM, LOGL_NOTICE, "RAND_bytes failed for A&C ref, falling "
> 559 "back to rand()\n");
> >>> CID 148206: Security best practices violations (DC.WEAK_CRYPTO)
> >>> "rand" should not be used for security related applications, as linear congruential algorithms are too easy to break.
well spotted, coverity :)
Do they actually analyse whether that it is security related code?
rand() is only the fallback for RAND_bytes().
~Neels
> 560 acreq->ac_ref_nr = rand();
> 561 } else
> 562 acreq->ac_ref_nr = rbyte;
> 563 mm->ac_ref_nr_used = acreq->ac_ref_nr;
> 564
> 565 /* Only if authentication is requested we need to set RAND + CKSN */
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZRgA0iChuO-2FGX0POK2RKsukBc1qHTwF2SHZpn6wOsolg-3D-3D_NeAJonveXivhzKyNx2umY31POArq17SmfipFEiTlbGEUEBBX9q2-2Ffp-2F8rVN3pZZhaIIJzuvyja9oX7zLeVod5CBloE2bNBsLMdq8vm8DwA4jz1vMIXKsvdJCo2dRiN0dLrWaTsM2iRtEWvVozsL8UDeM9yM6-2F2BIGt-2B16Y9D-2BQ8BbCg6SoNlU9hQz94ehIdlkNJbMmEwt26bsVaXK41zwg-3D-3D
>
> To manage Coverity Scan email notifications for "nhofmeyr at sysmocom.de", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRbVDbis712qZDP-2FA8y06Nq4VngPiVJb2v7ebXRF7QR8XkZ9r0tTrYp-2F0MW-2FEWllzpkyCC83LXF7q-2FHm2-2BquI0NN6y2ZgcVxPTvklWSJL2RR9R8Aq2Y2qnMdw30sEcvVsU8-3D_NeAJonveXivhzKyNx2umY31POArq17SmfipFEiTlbGEUEBBX9q2-2Ffp-2F8rVN3pZZhXX1scEo5tl4BXS4c8ZAYMpsh3qS3-2B6s0GpetAuas0BozvqlhP3mPN-2FpUsNIgsEs4-2FKN-2FWK3a4sXMQ-2FlZAsLPAq6wElxzIyBZHzt67dR8y8NfBpESjEzS5WBqF8WaJwKLMLGTnP4R4pKt5nYl889UXg-3D-3D
>
--
- Neels Hofmeyr <nhofmeyr at sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschäftsführer / Managing Directors: Harald Welte
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20160919/2465078b/attachment.bin>