From f04cdae7df7e9440b868d4ff7027dee29670a7b4 Mon Sep 17 00:00:00 2001 From: Till Varoquaux Date: Tue, 21 Oct 2014 18:55:56 -0400 Subject: [PATCH] Plug pub format_... security bug (only on glibc). --- acokws.m4 | 24 ++++- configure.in | 2 + librfn/filters.T | 158 +++++++++++++++++++++++++--- librfn/okrfn-int.h | 1 + librfn/rfn3.C | 1 + pub/pub3.T | 3 + test/pub/Makefile.am | 8 ++ test/pub/rfn3_format.expected | 4 + test/pub/rfn3_format.pub | 4 + test/pub/rfn3_format_wrong.error | 2 + test/pub/rfn3_format_wrong.expected | 2 + test/pub/rfn3_format_wrong.pub | 2 + 12 files changed, 198 insertions(+), 13 deletions(-) create mode 100644 test/pub/rfn3_format.expected create mode 100644 test/pub/rfn3_format.pub create mode 100644 test/pub/rfn3_format_wrong.error create mode 100644 test/pub/rfn3_format_wrong.expected create mode 100644 test/pub/rfn3_format_wrong.pub diff --git a/acokws.m4 b/acokws.m4 index e388759c..943e63e3 100644 --- a/acokws.m4 +++ b/acokws.m4 @@ -992,7 +992,29 @@ fi AM_CONDITIONAL(MAKE_OLD_FLEX, test ${current_flex} -eq 0) ]) - + +dnl +dnl checking for printf format parsing functions +dnl +AC_DEFUN([OKWS_PARSE_PRINTF_FORMAT], +[ + +AC_LANG_PUSH([C]) +AC_MSG_CHECKING(whether we have parse_printf_format) +AC_LINK_IFELSE( + [AC_LANG_PROGRAM( + [[#include ]], + [[parse_printf_format("%i abcd", 0, 0);]])], + [AC_MSG_RESULT(yes); + have_parse_printf_format=yes; + AC_DEFINE([HAVE_PARSE_PRINTF_FORMAT], + [1], + [Defined if we have the gnu parse_printf_format function])], + [AC_MSG_RESULT(no); have_parse_printf_format=no;]) +AC_LANG_POP([C]) +AM_CONDITIONAL([HAVE_PARSE_PRINTF_FORMAT], [test x$have_parse_printf_format = xyes]) +]) + dnl dnl Check for Linux prctl() to get coredumps after setuid/setgid dnl diff --git a/configure.in b/configure.in index 33dbd373..f7243a36 100644 --- a/configure.in +++ b/configure.in @@ -31,6 +31,8 @@ AC_PATH_PROG(PERL, perl, perl) AC_PATH_PROGS(M4, gm4 gnum4 m4, '$(top_srcdir)/missing') AC_PATH_PROGS(UPTIME, uptime, '$(top_srcdir)/missing') +OKWS_PARSE_PRINTF_FORMAT + dnl dnl On FreeBSD, look for PTH by default. However, PTH stopped working dnl on Linux with hard system calls, so use pthreads by default on diff --git a/librfn/filters.T b/librfn/filters.T index c0899dfd..539516a0 100644 --- a/librfn/filters.T +++ b/librfn/filters.T @@ -1,5 +1,6 @@ // -*-c++-*- +#include "config.h" #include "okrfn-int.h" #include "pubutil.h" #include "pescape.h" @@ -7,9 +8,67 @@ #include "crypt.h" #include "wide_str.h" #include "okws_rxx.h" +#include "sfs_assert.h" + +#ifdef HAVE_PARSE_PRINTF_FORMAT +extern "C" { +#include +} + +namespace { + // Check that a given string can be used as a format specifier for a printf + // like function. + int get_fmt1(const str fmt) { + int res; + if (!fmt) { + return PA_LAST; + } + const size_t len = parse_printf_format(fmt.cstr(), 1, &res); + if (len != 1) + return PA_LAST; + return res; + } +} // namespace +#endif // HAVE_PARSE_PRINTF_FORMAT namespace rfn3 { - + + // Allocates an `str` with the return value from `sprintf`. We are not using + // vuio_printf because we have no way to properly validate its input. + static str stprintf(const char *fmt, ...) { + int n; + constexpr size_t BUFFSIZ = 10; + char buff[BUFFSIZ]; + va_list ap; + + // First try with a stack allocated buffer. + va_start(ap, fmt); + n = vsnprintf(buff, BUFFSIZ, fmt, ap); + va_end(ap); + + // Failure + if (n < 0) { + DASSERT(false); + return nullptr; + } + + // The stack allocated buff was small enough... + if (n < BUFFSIZ) { + return str(buff, n); + } + + // Now we know the exact size.... + mstr res(n); + + va_start(ap, fmt); + n = vsnprintf(res.cstr(), res.len() + 1, fmt, ap); + va_end(ap); + + DASSERT(res.len() == n); + + return str(res); + } + //------------------------------------------------------------ static str @@ -300,12 +359,26 @@ a hexidecimal-encoded digest. ptr format_float_t::v_eval_2 (eval_t *p, const vec &args) const { -#define BUFSZ 1024 - char buf[BUFSZ]; str fmt = args[0]._s; - double d = args[1]._f; - snprintf (buf, BUFSZ, fmt.cstr (), d); - return expr_str_t::alloc (buf); + int spec = PA_FLOAT; + str res; +#ifdef HAVE_PARSE_PRINTF_FORMAT + spec = get_fmt1(fmt); +#endif + if (spec == PA_FLOAT) { + float f = args[1]._f; + res = stprintf(fmt.cstr (), f); + } else if (spec == (PA_DOUBLE)) { + double d = args[1]._f; + res = stprintf(fmt.cstr (), d); + } else if (spec == (PA_DOUBLE | PA_FLAG_LONG_DOUBLE)) { + long double ld = args[1]._f; + res = stprintf (fmt.cstr (), ld); + } else { + report_error(p, strbuf("Not a float format \"%s\" %i", fmt.cstr(), spec)); + return expr_null_t::alloc(); + } + return expr_str_t::alloc(res); } //------------------------------------------------------------ @@ -322,15 +395,34 @@ to the given sprintf-style format string `fmt`. ptr format_int_t::v_eval_2 (eval_t *p, const vec &args) const { - char buf[BUFSZ]; str fmt = args[0]._s; - int i = args[1]._i; - snprintf (buf, BUFSZ, fmt.cstr (), i); - return expr_str_t::alloc (buf); -#undef BUFSZ + str res; + int spec = PA_INT; + #ifdef HAVE_PARSE_PRINTF_FORMAT + spec = get_fmt1(fmt); + #endif + if (spec == PA_INT) { + int i = args[1]._i; + res = stprintf (fmt.cstr (), i); + } else if (spec == PA_CHAR) { + char c = args[1]._i; + res = stprintf (fmt.cstr (), c); + } else if (spec == (PA_INT | PA_FLAG_SHORT)) { + short s = args[1]._i; + res = stprintf (fmt.cstr (), s); + } else if (spec == (PA_INT | PA_FLAG_LONG)) { + long l = args[1]._i; + res = stprintf (fmt.cstr (), l); + } else if (spec == (PA_INT | PA_FLAG_LONG_LONG)) { + long long ll = args[1]._i; + res = stprintf (fmt.cstr (), ll); + } else { + report_error(p, strbuf("Not an int format \"%s\" %i", fmt.cstr(), spec)); + return expr_null_t::alloc(); + } + return expr_str_t::alloc (res); } - //------------------------------------------------------------ const str format_int_t::DOCUMENTATION = R"*(Format the int `val` according to @@ -342,6 +434,48 @@ the given sprintf-style format string `fmt`. //------------------------------------------------------------ + ptr + format_uint_t::v_eval_2 (eval_t *p, const vec &args) const + { + str res; + str fmt = args[0]._s; + int spec = PA_INT; + #ifdef HAVE_PARSE_PRINTF_FORMAT + spec = get_fmt1(fmt); + #endif + if (spec == PA_INT) { + unsigned int i = args[1]._u; + res = stprintf (fmt.cstr (), i); + } else if (spec == PA_CHAR) { + unsigned char c = args[1]._u; + res = stprintf (fmt.cstr (), c); + } else if (spec == (PA_INT | PA_FLAG_SHORT)) { + unsigned short s = args[1]._u; + res = stprintf (fmt.cstr (), s); + } else if (spec == (PA_INT | PA_FLAG_LONG)) { + unsigned long l = args[1]._u; + res = stprintf (fmt.cstr (), l); + } else if (spec == (PA_INT | PA_FLAG_LONG_LONG)) { + unsigned long long ll = args[1]._u; + res = stprintf (fmt.cstr (), ll); + } else { + report_error(p, strbuf("Not an int format \"%s\" %i", fmt.cstr(), spec)); + return expr_null_t::alloc(); + } + return expr_str_t::alloc (res); + } + + //------------------------------------------------------------ + + const str format_uint_t::DOCUMENTATION = R"*(Format the uint `val` according +to the given sprintf-style format string `fmt`. + +@param {string} fmt +@param {uint} val +@return {string})*"; + + //------------------------------------------------------------ + ptr json_escape_t::v_eval_2 (eval_t *p, const vec &args) const { diff --git a/librfn/okrfn-int.h b/librfn/okrfn-int.h index baa6df8c..e9bd19ac 100644 --- a/librfn/okrfn-int.h +++ b/librfn/okrfn-int.h @@ -76,6 +76,7 @@ namespace rfn3 { PUB3_COMPILED_FN_DOC(cmp, "OO"); PUB3_COMPILED_FN_DOC(format_float, "sf"); PUB3_COMPILED_FN_DOC(format_int, "si"); + PUB3_COMPILED_FN_DOC(format_uint, "su"); PUB3_COMPILED_FN_DOC(json_escape, "s"); PUB3_COMPILED_FN_DOC(js_escape, "s"); PUB3_COMPILED_FN_DOC(randsel, "l"); diff --git a/librfn/rfn3.C b/librfn/rfn3.C index 2c01991f..7b409284 100644 --- a/librfn/rfn3.C +++ b/librfn/rfn3.C @@ -100,6 +100,7 @@ numbers, JSON and time.)*"; F(cmp); F(format_float); F(format_int); + F(format_uint); F(stat_file); F(raw); F(utf8_fix); diff --git a/pub/pub3.T b/pub/pub3.T index 2d5dff53..17cac9b9 100644 --- a/pub/pub3.T +++ b/pub/pub3.T @@ -183,6 +183,9 @@ namespace { int main(int argc, char** argv) { cli_arguments_t cli; + make_sync(0); + make_sync(1); + make_sync(2); argp_parse (&argspecs, argc, argv, 0, 0, &cli); main2(cli.file, cli.jailed); amain(); diff --git a/test/pub/Makefile.am b/test/pub/Makefile.am index 6834bdb3..67d1f7f5 100644 --- a/test/pub/Makefile.am +++ b/test/pub/Makefile.am @@ -1,6 +1,7 @@ PACKAGE_STRING=pub TESTS = \ dict_equal.pub \ + rfn3_format.pub \ scoping_for.pub \ scoping_globals-locals.pub \ scoping_order_of_decl.pub \ @@ -12,8 +13,15 @@ TESTS = \ syntax_error.pub \ undef_vs_null.pub + if SFS_DEBUG TESTS+=blocking_call_warn.pub endif + +if HAVE_PARSE_PRINTF_FORMAT + TESTS+=rfn3_format_wrong.pub +endif + + AUTOMAKE_OPTIONS = parallel-tests LOG_COMPILER = $(builddir)/test_runner diff --git a/test/pub/rfn3_format.expected b/test/pub/rfn3_format.expected new file mode 100644 index 00000000..d7b751fc --- /dev/null +++ b/test/pub/rfn3_format.expected @@ -0,0 +1,4 @@ +2048 +0 +5123445645 +5.123 diff --git a/test/pub/rfn3_format.pub b/test/pub/rfn3_format.pub new file mode 100644 index 00000000..b783cb1e --- /dev/null +++ b/test/pub/rfn3_format.pub @@ -0,0 +1,4 @@ +%{len(format_uint("%.2048x", 5))} +%{format_uint("%hhx", 512)} +%{format_uint("%llu", 5123445645)} +%{format_float("%1$.3f", 5.1234456)} diff --git a/test/pub/rfn3_format_wrong.error b/test/pub/rfn3_format_wrong.error new file mode 100644 index 00000000..49238ceb --- /dev/null +++ b/test/pub/rfn3_format_wrong.error @@ -0,0 +1,2 @@ +okws-pub3[eval]: rfn3_format_wrong.pub:1: Not an int format "%s" 3 +okws-pub3[eval]: rfn3_format_wrong.pub:2: Not a float format "%3$i" 8 diff --git a/test/pub/rfn3_format_wrong.expected b/test/pub/rfn3_format_wrong.expected new file mode 100644 index 00000000..c1e4b6c1 --- /dev/null +++ b/test/pub/rfn3_format_wrong.expected @@ -0,0 +1,2 @@ +null +null diff --git a/test/pub/rfn3_format_wrong.pub b/test/pub/rfn3_format_wrong.pub new file mode 100644 index 00000000..3ef0b98b --- /dev/null +++ b/test/pub/rfn3_format_wrong.pub @@ -0,0 +1,2 @@ +%{rfn3.format_uint("%s", 5)} +%{rfn3.format_float("%3$i", 5.0)}