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 | 1 + src/vty/utils.c | 4 ++-- tests/Makefile.am | 9 +++++-- tests/testsuite.at | 6 +++++ tests/vty/vty_test.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/vty/vty_test.ok | 3 +++ 6 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/vty/vty_test.c create mode 100644 tests/vty/vty_test.ok diff --git a/.gitignore b/.gitignore index 2ed0144..da36165 100644 --- a/.gitignore +++ b/.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 diff --git a/src/vty/utils.c b/src/vty/utils.c index e9c0d2d..88932fa 100644 --- a/src/vty/utils.c +++ b/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); diff --git a/tests/Makefile.am b/tests/Makefile.am index e5fc718..ecb2b6c 100644 --- a/tests/Makefile.am +++ b/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 diff --git a/tests/testsuite.at b/tests/testsuite.at index 684ec4f..1a6fa55 100644 --- a/tests/testsuite.at +++ b/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 diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c new file mode 100644 index 0000000..6a53fb8 --- /dev/null +++ b/tests/vty/vty_test.c @@ -0,0 +1,62 @@ +/* (C) 2013 by Jacob Erlbeck <jerlbeck@osmocom.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 init_vty(void) +{ +} + +static void free_vty(void) +{ +} + +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) +{ + init_vty(); + test_cmd_string_from_valstr(); + printf("All tests passed\n"); + + free_vty(); + return 0; +} diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok new file mode 100644 index 0000000..9ff68c8 --- /dev/null +++ b/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 -- 1.7.9.5