On Mon, Sep 19, 2016 at 06:02:45AM -0700, scan-admin(a)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-2BfV0V0…
To manage Coverity Scan email notifications for "nhofmeyr(a)sysmocom.de"m.de", click
https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…
--
- Neels Hofmeyr <nhofmeyr(a)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