Plug pub format_... security bug (only on glibc).

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 <printf.h>]],
+      [[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

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

librfn/filters.T

@@ -1,15 +1,74 @@
 // -*-c++-*-
 
+#include "config.h"
 #include "okrfn-int.h"
 #include "pubutil.h"
 #include "pescape.h"
 #include "okcgi.h"
 #include "crypt.h"
 #include "wide_str.h"
 #include "okws_rxx.h"
+#include "sfs_assert.h"
+
+#ifdef HAVE_PARSE_PRINTF_FORMAT
+extern "C" {
+#include <printf.h>
+}
+
+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<const expr_t>
   format_float_t::v_eval_2 (eval_t *p, const vec<arg_t> &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<const expr_t>
   format_int_t::v_eval_2 (eval_t *p, const vec<arg_t> &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<const expr_t>
+  format_uint_t::v_eval_2 (eval_t *p, const vec<arg_t> &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<const expr_t>
   json_escape_t::v_eval_2 (eval_t *p, const vec<arg_t> &args) const
   {

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");

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);

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();

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

test/pub/rfn3_format.expected

@@ -0,0 +1,4 @@
+2048
+0
+5123445645
+5.123

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)}

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

test/pub/rfn3_format_wrong.expected

@@ -0,0 +1,2 @@
+null
+null

test/pub/rfn3_format_wrong.pub

@@ -0,0 +1,2 @@
+%{rfn3.format_uint("%s", 5)}
+%{rfn3.format_float("%3$i", 5.0)}