vty: Fix misusage of snprintf in vty/utils.c

Compiled with ubuntu 1204 (precise), where -Wformat-security is enabled by
-Wall.

Test yields ok, but the current implementation doesn't properly support
multi-character separators and end strings. So the test output is truncated.

Addresses:
utils.c: In function 'vty_cmd_string_from_valstr':
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]

.gitignore

@@ -72,6 +72,7 @@ tests/fr/fr_test
 tests/loggingrb/loggingrb_test
 tests/ringbuf/ringbuf_test
 tests/strrb/strrb_test
+tests/vty/vty_test
 
 utils/osmo-arfcn
 utils/osmo-auc-gen

src/vty/utils.c

@@ -81,7 +81,7 @@ char *vty_cmd_string_from_valstr(void *ctx, const struct value_string *vals,
 	if (!str)
 		return NULL;
 
-	ret = snprintf(str + offset, rem, prefix);
+	ret = snprintf(str + offset, rem, "%s", prefix);
 	if (ret < 0)
 		goto err;
 	OSMO_SNPRINTF_RET(ret, rem, offset, len);
@@ -105,7 +105,7 @@ char *vty_cmd_string_from_valstr(void *ctx, const struct value_string *vals,
 	offset--;	/* to remove the trailing | */
 	rem++;
 
-	ret = snprintf(str + offset, rem, end);
+	ret = snprintf(str + offset, rem, "%s", end);
 	if (ret < 0)
 		goto err;
 	OSMO_SNPRINTF_RET(ret, rem, offset, len);

tests/Makefile.am

@@ -5,7 +5,8 @@ check_PROGRAMS = timer/timer_test sms/sms_test ussd/ussd_test		\
                  conv/conv_test auth/milenage_test lapd/lapd_test	\
                  gsm0808/gsm0808_test gsm0408/gsm0408_test		\
 		 gb/bssgp_fc_test logging/logging_test fr/fr_test	\
-		 loggingrb/loggingrb_test strrb/strrb_test
+		 loggingrb/loggingrb_test strrb/strrb_test              \
+		 vty/vty_test
 
 if ENABLE_MSGFILE
 check_PROGRAMS += msgfile/msgfile_test
@@ -62,6 +63,9 @@ loggingrb_loggingrb_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_buildd
 strrb_strrb_test_SOURCES = strrb/strrb_test.c
 strrb_strrb_test_LDADD = $(top_builddir)/src/libosmocore.la
 
+vty_vty_test_SOURCES = vty/vty_test.c
+vty_vty_test_LDADD = $(top_builddir)/src/vty/libosmovty.la $(top_builddir)/src/libosmocore.la
+
 
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
 $(srcdir)/package.m4: $(top_srcdir)/configure.ac
@@ -91,7 +95,8 @@ EXTRA_DIST = testsuite.at $(srcdir)/package.m4 $(TESTSUITE)		\
              msgfile/msgfile_test.ok msgfile/msgconfig.cfg		\
              logging/logging_test.ok logging/logging_test.err		\
              fr/fr_test.ok loggingrb/logging_test.ok			\
-             loggingrb/logging_test.err	strrb/strrb_test.ok
+             loggingrb/logging_test.err	strrb/strrb_test.ok		\
+	     vty/vty_test.ok
 
 DISTCLEANFILES = atconfig
 

tests/testsuite.at

@@ -112,3 +112,9 @@ AT_KEYWORDS([strrb])
 cat $abs_srcdir/strrb/strrb_test.ok > expout
 AT_CHECK([$abs_top_builddir/tests/strrb/strrb_test], [0], [expout], [ignore])
 AT_CLEANUP
+
+AT_SETUP([vty])
+AT_KEYWORDS([vty])
+cat $abs_srcdir/vty/vty_test.ok > expout
+AT_CHECK([$abs_top_builddir/tests/vty/vty_test], [0], [expout], [ignore])
+AT_CLEANUP

tests/vty/vty_test.c

@@ -0,0 +1,52 @@
+/* (C) 2013 by Jacob Erlbeck <jerlbeck@sysmocom.de>
+ * All Rights Reserved
+ *
+ * This program is iree 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 3 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include <osmocom/core/talloc.h>
+#include <osmocom/core/logging.h>
+#include <osmocom/core/utils.h>
+#include <osmocom/vty/misc.h>
+
+static void test_cmd_string_from_valstr(void)
+{
+	char *cmd;
+	const struct value_string printf_seq_vs[] = {
+		{ .value = 42, .str = "[foo%s%s%s%s%s]"},
+		{ .value = 43, .str = "[bar%s%s%s%s%s]"},
+		{ .value = 0,  .str = NULL}
+	};
+
+	printf("Going to test vty_cmd_string_from_valstr()\n");
+
+	/* check against character strings that could break printf */
+
+	cmd = vty_cmd_string_from_valstr (NULL, printf_seq_vs, "[prefix%s%s%s%s%s]", "[sep%s%s%s%s%s]", "[end%s%s%s%s%s]", 1);
+	printf ("Tested with %%s-strings, resulting cmd = '%s'\n", cmd);
+	talloc_free (cmd);
+}
+
+int main(int argc, char **argv)
+{
+	test_cmd_string_from_valstr();
+	printf("All tests passed\n");
+
+	return 0;
+}

tests/vty/vty_test.ok

@@ -0,0 +1,3 @@
+Going to test vty_cmd_string_from_valstr()
+Tested with %s-strings, resulting cmd = '[prefix%s%s%s%s%s][foo%s%s%s%s%s][sep%s%s%s%s%s]['
+All tests passed