[PATCH 3/4] nitb: Check source string length before calling strncpy (Coverity)

[Jacob Erlbeck]

Currently some VTY command do neither check the length of the source
string before calling strncpy nor ensure NUL-termination afterwards.
This can to destination string buffers whose contents are not
NUL-teminated.

This commit adds checks and corresponding warnings to the VTY
commands 'subscriber TYPE ID name .NAME" and "subscriber TYPE ID
extension EXTENSION".

Fixes: Coverity CID 1206570, 1206569
Sponsored-by: On-Waves ehf
---
 openbsc/src/libmsc/vty_interface_layer3.c | 14 ++++++++++++++
 openbsc/tests/vty_test_runner.py          | 26 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c
index 68d9c44..558db5e 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -548,6 +548,13 @@ DEFUN(ena_subscr_name,
 		return CMD_WARNING;
 	}
 
+	if (strlen(name) > sizeof(subscr->name)-1) {
+		vty_out(vty,
+			"%% NAME is too long, max. %d characters are allowed%s",
+			sizeof(subscr->name)-1, VTY_NEWLINE);
+		return CMD_WARNING;
+	}
+
 	strncpy(subscr->name, name, sizeof(subscr->name));
 	talloc_free(name);
 	db_sync_subscriber(subscr);
@@ -574,6 +581,13 @@ DEFUN(ena_subscr_extension,
 		return CMD_WARNING;
 	}
 
+	if (strlen(ext) > sizeof(subscr->extension)-1) {
+		vty_out(vty,
+			"%% EXTENSION is too long, max. %d characters are allowed%s",
+			sizeof(subscr->extension)-1, VTY_NEWLINE);
+		return CMD_WARNING;
+	}
+
 	strncpy(subscr->extension, ext, sizeof(subscr->extension));
 	db_sync_subscriber(subscr);
 
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index 2b7fd19..fb4ca7d 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -286,6 +286,32 @@ class TestVTYNITB(TestVTYGenericBSC):
         res = self.vty.command('show subscriber imsi '+imsi)
         self.assert_(res != '% No subscriber found for imsi '+imsi)
 
+    def testSubscriberSettings(self):
+        self.vty.enable()
+
+        imsi = "204300854013739"
+        wrong_imsi = "204300999999999"
+
+        # Lets create one
+        res = self.vty.command('subscriber create imsi '+imsi)
+        self.assert_(res.find("    IMSI: "+imsi) > 0)
+
+        self.vty.verify('subscriber imsi '+wrong_imsi+' name wrong', ['% No subscriber found for imsi '+wrong_imsi])
+        res = self.vty.command('subscriber imsi '+imsi+' name '+('X' * 160))
+        self.assert_(res.find("NAME is too long") > 0)
+
+        self.vty.verify('subscriber imsi '+imsi+' name '+('G' * 159), [''])
+
+        self.vty.verify('subscriber imsi '+wrong_imsi+' extension 840', ['% No subscriber found for imsi '+wrong_imsi])
+        res = self.vty.command('subscriber imsi '+imsi+' extension '+('9' * 15))
+        self.assert_(res.find("EXTENSION is too long") > 0)
+
+        self.vty.verify('subscriber imsi '+imsi+' extension '+('1' * 14), [''])
+
+        # Delete it
+        res = self.vty.command('subscriber delete imsi '+imsi)
+        self.assert_(res != "")
+
     def testShowPagingGroup(self):
         res = self.vty.command("show paging-group 255 1234567")
         self.assertEqual(res, "% can't find BTS 255")
-- 
1.9.1