From 259e797a8abed7bbf5de502d0eaa4634ce1bf5cd Mon Sep 17 00:00:00 2001
From: Vadim Yanitskiy <vyanitskiy@sysmocom.de>
Date: Thu, 14 Jan 2021 03:31:29 +0100
Subject: [PATCH] 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
---
 src/osmo-bsc/bsc_vty.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 753acf7ab..5b4417ef0 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -6820,6 +6820,17 @@ DEFUN_USRATTR(cfg_net_bsc_codec_list,
 	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 @@ DEFUN_USRATTR(cfg_net_bsc_codec_list,
 	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);