Change in osmo-bsc[master]: osmo_bsc_ctrl: make sure strtok results are checked

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/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Nov 11 09:00:39 UTC 2021


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26127 )

Change subject: osmo_bsc_ctrl: make sure strtok results are checked
......................................................................

osmo_bsc_ctrl: make sure strtok results are checked

The function set_bts_loc does not check the string pointers resturned by
strtok_r. In this particular case this is not a problem because the
function set_bts_lock will only see verfied input. However, lets check
the results anyway to avoid creating false positives in coverity scan.

Change-Id: Ie21c392e0405fc45811c6d55bf5508e9eb6784de
Fixes: CID#240849
---
M src/osmo-bsc/osmo_bsc_ctrl.c
1 file changed, 14 insertions(+), 7 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  osmith: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/osmo-bsc/osmo_bsc_ctrl.c b/src/osmo-bsc/osmo_bsc_ctrl.c
index 1eea690..969efb5 100644
--- a/src/osmo-bsc/osmo_bsc_ctrl.c
+++ b/src/osmo-bsc/osmo_bsc_ctrl.c
@@ -428,6 +428,20 @@
 	if (!tmp)
 		goto oom;
 
+	tstamp = strtok_r(tmp, ",", &saveptr);
+	valid = strtok_r(NULL, ",", &saveptr);
+	lat = strtok_r(NULL, ",", &saveptr);
+	lon = strtok_r(NULL, ",", &saveptr);
+	height = strtok_r(NULL, "\0", &saveptr);
+
+	/* Check if one of the strtok results was NULL. This will probably never occur since we will only see verified
+	 * input in this code path */
+	if ((tstamp == NULL) || (valid == NULL) || (lat == NULL) || (lon == NULL) || (height == NULL)) {
+		talloc_free(tmp);
+		cmd->reply = "parse error";
+		return CTRL_CMD_ERROR;
+	}
+
 	curloc = talloc_zero(tall_bsc_ctx, struct bts_location);
 	if (!curloc) {
 		talloc_free(tmp);
@@ -435,13 +449,6 @@
 	}
 	INIT_LLIST_HEAD(&curloc->list);
 
-
-	tstamp = strtok_r(tmp, ",", &saveptr);
-	valid = strtok_r(NULL, ",", &saveptr);
-	lat = strtok_r(NULL, ",", &saveptr);
-	lon = strtok_r(NULL, ",", &saveptr);
-	height = strtok_r(NULL, "\0", &saveptr);
-
 	curloc->tstamp = atol(tstamp);
 	curloc->valid = get_string_value(bts_loc_fix_names, valid);
 	curloc->lat = atof(lat);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26127
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ie21c392e0405fc45811c6d55bf5508e9eb6784de
Gerrit-Change-Number: 26127
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211111/fbca6a61/attachment.htm>


More information about the gerrit-log mailing list