diff options
authorJacob Erlbeck <>2015-04-09 14:47:18 +0200
committerHolger Hans Peter Freyther <>2015-04-10 08:47:00 +0200
commit7ffa7b095fbea0f7f52a4179402edb04249c60d0 (patch)
parent322b1499cd4d34b0148a15cb615ad6dff8203ed2 (diff)
nitb: Fix IMSI/IMEI buffer handling (Coverity)
Currently the handling of the buffers is not done consistently. Some code assumes that the whole buffer may be used to store the string while at other places, the last buffer byte is left untouched in the assumption that it contains a terminating NUL-character. The latter is the correct behaviour. This commit changes to code to not touch the last byte in the buffers and to rely on the last byte being NUL. So the maximum IMSI/IMEI length is GSM_IMSI_LENGTH-1/GSM_IMEI_LENGTH-1. For information: We assume that we allocate the structure with talloc_zero. This means we have NULed the entire imsi array and then only write sizeof - 1 characters to it. So the last byte remains NUL. Fixes: Coverity CID 1206568, 1206567 Sponsored-by: On-Waves ehf
3 files changed, 4 insertions, 5 deletions
diff --git a/openbsc/src/libcommon/gsm_subscriber_base.c b/openbsc/src/libcommon/gsm_subscriber_base.c
index 3c56101f6..a455824a3 100644
--- a/openbsc/src/libcommon/gsm_subscriber_base.c
+++ b/openbsc/src/libcommon/gsm_subscriber_base.c
@@ -112,8 +112,7 @@ struct gsm_subscriber *subscr_get_or_create(struct gsm_subscriber_group *sgrp,
if (!subscr)
return NULL;
- strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH);
- subscr->imsi[GSM_IMSI_LENGTH - 1] = '\0';
+ strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH-1);
subscr->group = sgrp;
return subscr;
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index bdecbb436..428f99bbc 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -565,7 +565,7 @@ static int get_equipment_by_subscr(struct gsm_subscriber *subscr)
string = dbi_result_get_string(result, "imei");
if (string)
- strncpy(equip->imei, string, sizeof(equip->imei));
+ strncpy(equip->imei, string, sizeof(equip->imei)-1);
string = dbi_result_get_string(result, "classmark1");
if (string) {
@@ -802,7 +802,7 @@ static void db_set_from_query(struct gsm_subscriber *subscr, dbi_conn result)
const char *string;
string = dbi_result_get_string(result, "imsi");
if (string)
- strncpy(subscr->imsi, string, GSM_IMSI_LENGTH);
+ strncpy(subscr->imsi, string, GSM_IMSI_LENGTH-1);
string = dbi_result_get_string(result, "tmsi");
if (string)
diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
index ac5a9f5c5..67844b812 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
@@ -399,7 +399,7 @@ int bsc_ussd_check(struct nat_sccp_connection *con, struct bsc_nat_parsed *parse
if (parsed->bssap != BSSAP_MSG_DTAP)
return 0;
- if (strlen(con->imsi) > GSM_IMSI_LENGTH)
+ if (strlen(con->imsi) >= GSM_IMSI_LENGTH)
return 0;
hdr48 = bsc_unpack_dtap(parsed, msg, &len);