cbsp: Fix decoding of Fail List

The decoding pointer was not increased correctly, ending up in reading
by 1 byte offset  for each item in the list.

Change-Id: I16ed9bd65109a7ce32ff43c5789b4544479838e7

5 files changed, 125 insertions, 2 deletions

diff --git a/src/gsm/cbsp.c b/src/gsm/cbsp.c
index 81ba81e2..7411056f 100644
--- a/src/gsm/cbsp.c
+++ b/src/gsm/cbsp.c
@@ -537,8 +537,8 @@ static int cbsp_decode_fail_list(struct llist_head *fl, void *ctx,
 		struct osmo_cbsp_fail_ent *ent = talloc_zero(ctx, struct osmo_cbsp_fail_ent);
 		unsigned int len_remain = len - (cur - buf);
 		OSMO_ASSERT(ent);
-		ent->id_discr = cur[0];
-		rc = gsm0808_decode_cell_id_u(&ent->cell_id, ent->id_discr, cur+1, len_remain-1);
+		ent->id_discr = *cur++;
+		rc = gsm0808_decode_cell_id_u(&ent->cell_id, ent->id_discr, cur, len_remain-1);
 		if (rc < 0) {
 			osmo_cbsp_errstr = "fail list: error decoding cell_id_union";
 			return rc;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b5448c01..8c7e40a0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -48,6 +48,7 @@ check_PROGRAMS = timer/timer_test sms/sms_test ussd/ussd_test		\
 		 iuup/iuup_test						\
 		 smscb/smscb_test                                       \
 		 smscb/gsm0341_test                                     \
+		 smscb/cbsp_test                                        \
 		 $(NULL)
 
 if ENABLE_MSGFILE
@@ -171,6 +172,9 @@ smscb_smscb_test_LDADD = $(LDADD) $(top_builddir)/src/gsm/libosmogsm.la
 smscb_gsm0341_test_SOURCES = smscb/gsm0341_test.c
 smscb_gsm0341_test_LDADD = $(LDADD) $(top_builddir)/src/gsm/libosmogsm.la
 
+smscb_cbsp_test_SOURCES = smscb/cbsp_test.c
+smscb_cbsp_test_LDADD = $(LDADD) $(top_builddir)/src/gsm/libosmogsm.la
+
 sms_sms_test_SOURCES = sms/sms_test.c
 sms_sms_test_LDADD = $(LDADD) $(top_builddir)/src/gsm/libosmogsm.la
 
@@ -430,6 +434,7 @@ EXTRA_DIST = testsuite.at $(srcdir)/package.m4 $(TESTSUITE)		\
 	     iuup/iuup_test.ok \
 	     smscb/smscb_test.ok \
 	     smscb/gsm0341_test.ok \
+	     smscb/cbsp_test.ok \
 	     $(NULL)
 
 if ENABLE_LIBSCTP
@@ -486,6 +491,8 @@ endif
 		>$(srcdir)/smscb/smscb_test.ok
 	smscb/gsm0341_test \
 		>$(srcdir)/smscb/gsm0341_test.ok
+	smscb/cbsp_test \
+		>$(srcdir)/smscb/cbsp_test.ok
 	ussd/ussd_test \
 		>$(srcdir)/ussd/ussd_test.ok
 	auth/milenage_test \
diff --git a/tests/smscb/cbsp_test.c b/tests/smscb/cbsp_test.c
new file mode 100644
index 00000000..d530798d
--- /dev/null
+++ b/tests/smscb/cbsp_test.c
@@ -0,0 +1,108 @@
+/*
+ * (C) 2022 by sysmocom - s.f.m.c. GmbH
+ * All Rights Reserved
+ *
+ * Author: Pau Espin Pedrol <pespin@sysmocom.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <osmocom/core/utils.h>
+#include <osmocom/core/msgb.h>
+
+#include <osmocom/gsm/protocol/gsm_48_049.h>
+#include <osmocom/gsm/cbsp.h>
+
+/*
+CBSP WRITE-REPLACE FAILURE
+ Message Type: WRITE-REPLACE FAILURE (3)
+ Message Length: 44
+ IE: Message Identifier: 0x0031
+  Information Element Identifier: Message Identifier (14)
+  Message Identifier: 0x0031
+ IE: New Serial Number: 0x4170
+  Information Element Identifier: New Serial Number (3)
+  New Serial Number: 0x4170
+ IE: Failure List: 2 items
+  Information Element Identifier: Failure List (9)
+  Information Element Length: 15
+  Failure List Item: MCC 901 International Mobile, shared code, MNC 70 Clementvale Baltic OÜ, LAC 0x0018, CI 0x0030: Cause Cell-identity-not-valid
+   Cell ID Discriminator: CGI (0)
+   Mobile Country Code (MCC): International Mobile, shared code (901)
+   Mobile Network Code (MNC): Clementvale Baltic OÜ (70)
+   Location Area Code (LAC): 0x0018
+   Cell Identifier (CI): 0x0030
+   Cause: Cell-identity-not-valid (0x03)
+  Failure List Item: LAC 02711, CI 0xc351: Cause LAI-or-LAC-not-valid
+   Cell ID Discriminator: LAC+CI (1)
+   Location Area Code (LAC): 0x2711
+   Cell Identifier (CI): 0xc351
+   Cause: LAI-or-LAC-not-valid (0x0f)
+ IE: Cell List (CGI): 2 items
+  Information Element Identifier: Cell List (4)
+  Information Element Length: 15
+  Cell ID Discriminator: CGI (0)
+  Cell List Item: MCC 901 International Mobile, shared code, MNC 70 Clementvale Baltic OÜ, LAC 0x0017, CI 0x002a
+   Mobile Country Code (MCC): International Mobile, shared code (901)
+   Mobile Network Code (MNC): Clementvale Baltic OÜ (70)
+   Location Area Code (LAC): 0x0017
+   Cell Identifier (CI): 0x002a
+  Cell List Item: MCC 901 International Mobile, shared code, MNC 70 Clementvale Baltic OÜ, LAC 0x0018, CI 0x002a
+   Mobile Country Code (MCC): International Mobile, shared code (901)
+   Mobile Network Code (MNC): Clementvale Baltic OÜ (70)
+   Location Area Code (LAC): 0x0018
+   Cell Identifier (CI): 0x002a
+ IE: Channel Indicator: basic channel
+  Information Element Identifier: Channel Indicator (18)
+  Channel Indicator: basic channel (0x00)
+*/
+static const char write_repl_fail_with_failure_list[] =
+	"0300002c0e003103417009000f0009f1070018003003012711c3510f04000f0009f1070017002a09f1070018002a1200";
+
+static struct msgb *msgb_from_hex(unsigned int size, const char *hex)
+{
+	struct msgb *msg = msgb_alloc(size, "test_cbsp");
+	OSMO_ASSERT(msg);
+	msg->l1h = msgb_put(msg, osmo_hexparse(hex, msg->data, msgb_tailroom(msg)));
+	msg->l2h = msg->l1h + sizeof(struct cbsp_header);
+	return msg;
+}
+
+static void test_decode()
+{
+	struct msgb *msg;
+	struct osmo_cbsp_decoded *cbsp_dec;
+
+	printf("=== %s start ===\n", __func__);
+
+	msg = msgb_from_hex(sizeof(write_repl_fail_with_failure_list),
+			    write_repl_fail_with_failure_list);
+
+	cbsp_dec = osmo_cbsp_decode(NULL, msg);
+	OSMO_ASSERT(cbsp_dec);
+
+	talloc_free(cbsp_dec);
+	msgb_free(msg);
+
+	printf("=== %s end ===\n", __func__);
+}
+
+int main(int argc, char **argv)
+{
+	test_decode();
+
+	return EXIT_SUCCESS;
+}
diff --git a/tests/smscb/cbsp_test.ok b/tests/smscb/cbsp_test.ok
new file mode 100644
index 00000000..a837de57
--- /dev/null
+++ b/tests/smscb/cbsp_test.ok
@@ -0,0 +1,2 @@
+=== test_decode start ===
+=== test_decode end ===
diff --git a/tests/testsuite.at b/tests/testsuite.at
index cc0b8c33..6b2e75f9 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -108,6 +108,12 @@ cat $abs_srcdir/smscb/gsm0341_test.ok > expout
 AT_CHECK([$abs_top_builddir/tests/smscb/gsm0341_test], [0], [expout])
 AT_CLEANUP
 
+AT_SETUP([smscb_cbsp])
+AT_KEYWORDS([smscb_cbsp])
+cat $abs_srcdir/smscb/cbsp_test.ok > expout
+AT_CHECK([$abs_top_builddir/tests/smscb/cbsp_test], [0], [expout])
+AT_CLEANUP
+
 AT_SETUP([ussd])
 AT_KEYWORDS([ussd])
 cat $abs_srcdir/ussd/ussd_test.ok > expout