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

[fixeria]

fixeria has uploaded this change for review. ( 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(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/67/22167/1

diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 400e1a6..fb9083b 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -6825,6 +6825,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);
@@ -6838,14 +6849,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: 1
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210114/c387df00/attachment.htm>