Change in osmo-bsc[master]: vty: fix 'codec-list' command: check all given arguments first

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/.

fixeria gerrit-no-reply at lists.osmocom.org
Thu Jan 14 20:47:09 UTC 2021


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

Change subject: vty: fix 'codec-list' command: check all given arguments first
......................................................................

vty: fix 'codec-list' command: check all given arguments first

Allocating a new list of supported codecs *before* checking the
command arguments is a bad idea.  The operator may simply mistype
one of the codecs and will end up with a list of NULL pointers.

The functions calling audio_support_to_gsm88() assume that this
list always does contain valid pointers, so if a new subscriber
connection gets established, or the operator simply invokes
'show running-config', osmo-bsc would crash due to NULL pointer
dereference.

Steps to reproduce:

  1. In the VTY, do: 'en' -> 'configure terminal' -> 'msc';
  2. Configure any invalid codec list, e.g. 'codec-list Boom!';
  3. Invoke 'show running-config', boom!

Let's check the input before changing the internal structures.

Change-Id: I35b740a39c9cf3716d286e717486ef505bc61522
Fixes: OS#4946
---
M src/osmo-bsc/bsc_vty.c
1 file changed, 11 insertions(+), 8 deletions(-)

Approvals:
  fixeria: Looks good to me, approved
  lynxis lazus: Looks good to me, but someone else must approve
  osmith: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 753acf7..5b4417e 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -6820,6 +6820,17 @@
 	struct bsc_msc_data *data = bsc_msc_data(vty);
 	int i;
 
+	/* check all given arguments first */
+	for (i = 0; i < argc; ++i) {
+		/* check for hrX or frX */
+		if (strlen(argv[i]) != 3
+				|| argv[i][1] != 'r'
+				|| (argv[i][0] != 'h' && argv[i][0] != 'f')
+				|| argv[i][2] < 0x30
+				|| argv[i][2] > 0x39)
+			goto error;
+	}
+
 	/* free the old list... if it exists */
 	if (data->audio_support) {
 		talloc_free(data->audio_support);
@@ -6833,14 +6844,6 @@
 	data->audio_length = argc;
 
 	for (i = 0; i < argc; ++i) {
-		/* check for hrX or frX */
-		if (strlen(argv[i]) != 3
-				|| argv[i][1] != 'r'
-				|| (argv[i][0] != 'h' && argv[i][0] != 'f')
-				|| argv[i][2] < 0x30
-				|| argv[i][2] > 0x39)
-			goto error;
-
 		data->audio_support[i] = talloc_zero(data->audio_support,
 				struct gsm_audio_support);
 		data->audio_support[i]->ver = atoi(argv[i] + 2);

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I35b740a39c9cf3716d286e717486ef505bc61522
Gerrit-Change-Number: 22167
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210114/1da793b1/attachment.htm>


More information about the gerrit-log mailing list