From 05d657339a80405900f8a066e41f08c121755cbf Mon Sep 17 00:00:00 2001
From: Stef Walter <stefw@redhat.com>
Date: Fri, 3 Jan 2014 16:37:12 +0100
Subject: [PATCH] sssd: Use safe printf when formatting full_name_format
strings
Since sssd.conf uses such printf formats in its code, and it's
completely bogus to be passing user provided input to printf
directly...
---
service/Makefile.am | 1 +
service/realm-sssd.c | 52 ++++---
service/safe-printf.c | 348 +++++++++++++++++++++++++++++++++++++++++++++++
service/safe-printf.h | 45 ++++++
tests/Makefile.am | 6 +
tests/test-safe-printf.c | 238 ++++++++++++++++++++++++++++++++
7 files changed, 662 insertions(+), 29 deletions(-)
create mode 100644 service/safe-printf.c
create mode 100644 service/safe-printf.h
create mode 100644 tests/test-safe-printf.c
diff --git a/service/Makefile.am b/service/Makefile.am
index 37743a7..603ccbe 100644
--- a/service/Makefile.am
+++ b/service/Makefile.am
@@ -59,6 +59,7 @@ realmd_SOURCES = \
realm-sssd-config.c realm-sssd-config.h \
realm-sssd-ipa.c realm-sssd-ipa.h \
realm-usleep-async.c realm-usleep-async.h \
+ safe-printf.c safe-printf.h \
$(NULL)
realmd_CFLAGS = \
diff --git a/service/realm-sssd.c b/service/realm-sssd.c
index b904514..a4863d4 100644
--- a/service/realm-sssd.c
+++ b/service/realm-sssd.c
@@ -25,6 +25,7 @@
#include "realm-service.h"
#include "realm-sssd.h"
#include "realm-sssd-config.h"
+#include "safe-printf.h"
#include <glib/gstdio.h>
#include <glib/gi18n.h>
@@ -315,38 +316,13 @@ update_domain (RealmSssd *self)
g_free (domain);
}
-static gchar *
-build_login_format (const gchar *format,
- ...) G_GNUC_PRINTF (1, 2);
-
-static gchar *
-build_login_format (const gchar *format,
- ...)
-{
- gchar *result;
- va_list va;
-
- /* This function exists mostly to get around gcc warnings */
-
- if (format == NULL)
- format = "%1$s@%2$s";
-
- va_start (va, format);
- result = g_strdup_vprintf (format, va);
- va_end (va);
-
- return result;
-}
-
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wformat-nonliteral"
-
static void
update_login_formats (RealmSssd *self)
{
RealmKerberos *kerberos = REALM_KERBEROS (self);
gchar *login_formats[2] = { NULL, NULL };
gchar *format = NULL;
+ gchar *domain_name;
gboolean qualify;
if (self->pv->section == NULL) {
@@ -366,10 +342,28 @@ update_login_formats (RealmSssd *self)
/* Setup the login formats */
format = realm_ini_config_get (self->pv->config, self->pv->section, "full_name_format");
- /* Here we place a '%s' in the place of the user in the format */
- login_formats[0] = build_login_format (format, "%U", self->pv->domain);
- realm_kerberos_set_login_formats (kerberos, (const gchar **)login_formats);
+ /* The full domain name */
+ domain_name = calc_domain (self);
+
+ /*
+ * In theory we should be discovering the short name or flat name as sssd
+ * calls it. We configured it as the sssd.conf 'domains' name, so we just
+ * use that. Eventually we want to have a way to query sssd for that.
+ */
+
+ /*
+ * Here we place a '%U' in the place of the user in the format, and
+ * fill in the domain appropriately. sssd uses snprintf for this, which
+ * is risky and very compex to do right with positional arguments.
+ *
+ * We only replace the arguments documented in sssd.conf, as well as
+ * other non-field printf replacements.
+ */
+
+ if (safe_asprintf (login_formats, format, "%U", domain_name ? domain_name : "", self->pv->domain, NULL) >= 0)
+ realm_kerberos_set_login_formats (kerberos, (const gchar **)login_formats);
g_free (login_formats[0]);
+ g_free (domain_name);
g_free (format);
}
diff --git a/service/safe-printf.c b/service/safe-printf.c
new file mode 100644
index 0000000..069c915
--- /dev/null
+++ b/service/safe-printf.c
@@ -0,0 +1,348 @@
+/* realmd -- Realm configuration service
+ *
+ * Copyright 2013 Red Hat Inc
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2 of the licence or (at
+ * your option) any later version.
+ *
+ * See the included COPYING file for more information.
+ *
+ * Author: Stef Walter <stefw@redhat.com>
+ */
+
+#include "config.h"
+
+#include "safe-printf.h"
+
+#include <stdarg.h>
+#include <string.h>
+
+#ifndef MIN
+#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#endif
+
+#ifndef MAX
+#define MAX(a, b) (((a) > (b)) ? (a) : (b))
+#endif
+
+static void
+safe_padding (int count,
+ int *total,
+ void (* callback) (void *, const char *, size_t),
+ void *data)
+{
+ char eight[] = " ";
+ int num;
+
+ while (count > 0) {
+ num = MIN (count, 8);
+ callback (data, eight, num);
+ count -= num;
+ *total += num;
+ }
+}
+
+static void
+dummy_callback (void *data,
+ const char *piece,
+ size_t len)
+{
+
+}
+
+int
+safe_printf_cb (void (* callback) (void *, const char *, size_t),
+ void *data,
+ const char *format,
+ const char * const args[],
+ int num_args)
+{
+ int at_arg = 0;
+ const char *cp;
+ int precision;
+ int width;
+ int len;
+ const char *value;
+ int total;
+ int left;
+
+ if (!callback)
+ callback = dummy_callback;
+
+ total = 0;
+ cp = format;
+
+ while (*cp) {
+
+ /* Piece of raw string */
+ if (*cp != '%') {
+ len = strcspn (cp, "%");
+ callback (data, cp, len);
+ total += len;
+ cp += len;
+ continue;
+ }
+
+ cp++;
+
+ /* An literal percent sign? */
+ if (*cp == '%') {
+ callback (data, "%", 1);
+ total++;
+ cp++;
+ continue;
+ }
+
+ value = NULL;
+ left = 0;
+ precision = -1;
+ width = -1;
+
+ /* Test for positional argument. */
+ if (*cp >= '0' && *cp <= '9') {
+ /* Look-ahead parsing, otherwise skipped */
+ if (cp[strspn (cp, "0123456789")] == '$') {
+ unsigned int n = 0;
+ for (; *cp >= '0' && *cp <= '9'; cp++)
+ n = 10 * n + (*cp - '0');
+ /* Positional argument 0 is invalid. */
+ if (n == 0)
+ return -1;
+ /* Positional argument N too high */
+ if (n > num_args)
+ return -1;
+ value = args[n - 1];
+ cp++; /* $ */
+ }
+ }
+
+ /* Read the supported flags. */
+ for (; ; cp++) {
+ if (*cp == '-')
+ left = 1;
+ /* Supported but ignored */
+ else if (*cp != ' ')
+ break;
+ }
+
+ /* Parse the width. */
+ if (*cp >= '0' && *cp <= '9') {
+ width = 0;
+ for (; *cp >= '0' && *cp <= '9'; cp++)
+ width = 10 * width + (*cp - '0');
+ }
+
+ /* Parse the precision. */
+ if (*cp == '.') {
+ precision = 0;
+ for (cp++; *cp >= '0' && *cp <= '9'; cp++)
+ precision = 10 * precision + (*cp - '0');
+ }
+
+ /* Read the conversion character. */
+ switch (*cp++) {
+ case 's':
+ /* Non-positional argument */
+ if (value == NULL) {
+ /* Too many arguments used */
+ if (at_arg == num_args)
+ return -1;
+ value = args[at_arg++];
+ }
+ break;
+
+ /* No other conversion characters are supported */
+ default:
+ return -1;
+ }
+
+ /* How many characters are we printing? */
+ len = strlen (value);
+ if (precision >= 0)
+ len = MIN (precision, len);
+
+ /* Do we need padding? */
+ safe_padding (left ? 0 : width - len, &total, callback, data);
+
+ /* The actual data */;
+ callback (data, value, len);
+ total += len;
+
+ /* Do we need padding? */
+ safe_padding (left ? width - len : 0, &total, callback, data);
+ }
+
+ return total;
+}
+
+struct sprintf_ctx {
+ char *data;
+ size_t length;
+ size_t alloc;
+};
+
+static void
+asprintf_callback (void *data,
+ const char *piece,
+ size_t length)
+{
+ struct sprintf_ctx *cx = data;
+ void *mem;
+
+ if (!cx->data)
+ return;
+
+ /* Reallocate if necessary */
+ if (cx->length + length + 1 > cx->alloc) {
+ cx->alloc += MAX (length + 1, 1024);
+ mem = realloc (cx->data, cx->alloc);
+ if (mem == NULL) {
+ free (cx->data);
+ cx->data = NULL;
+ return;
+ }
+ cx->data = mem;
+ }
+
+ memcpy (cx->data + cx->length, piece, length);
+ cx->length += length;
+ cx->data[cx->length] = 0;
+}
+
+static const char **
+valist_to_args (va_list va,
+ int *num_args)
+{
+ int alo_args;
+ const char **args;
+ const char *arg;
+ void *mem;
+
+ *num_args = alo_args = 0;
+ args = NULL;
+
+ for (;;) {
+ arg = va_arg (va, const char *);
+ if (arg == NULL)
+ break;
+ if (*num_args == alo_args) {
+ alo_args += 8;
+ mem = realloc (args, sizeof (const char *) * alo_args);
+ if (!mem) {
+ free (args);
+ return NULL;
+ }
+ args = mem;
+ }
+ args[(*num_args)++] = arg;
+ }
+
+ return args;
+}
+
+int
+safe_asprintf (char **strp,
+ const char *format,
+ ...)
+{
+ struct sprintf_ctx cx;
+ const char **args;
+ int num_args;
+ va_list va;
+ int ret;
+ int i;
+
+ va_start (va, format);
+ args = valist_to_args (va, &num_args);
+ va_end (va);
+
+ if (args == NULL)
+ return -1;
+
+ /* Preallocate a pretty good guess */
+ cx.alloc = strlen (format) + 1;
+ for (i = 0; i < num_args; i++)
+ cx.alloc += strlen (args[i]);
+
+ cx.data = malloc (cx.alloc);
+ if (!cx.data) {
+ free (args);
+ return -1;
+ }
+
+ cx.data[0] = '\0';
+ cx.length = 0;
+
+ ret = safe_printf_cb (asprintf_callback, &cx, format, args, num_args);
+ if (cx.data == NULL)
+ ret = -1;
+ if (ret < 0)
+ free (cx.data);
+ else
+ *strp = cx.data;
+
+ free (args);
+ return ret;
+}
+
+static void
+snprintf_callback (void *data,
+ const char *piece,
+ size_t length)
+{
+ struct sprintf_ctx *cx = data;
+
+ /* Don't copy if too much data */
+ if (cx->length > cx->alloc)
+ length = 0;
+ else if (cx->length + length > cx->alloc)
+ length = cx->alloc - cx->length;
+ else
+ length = length;
+
+ if (length > 0)
+ memcpy (cx->data + cx->length, piece, length);
+
+ /* Null termination happens later */
+ cx->length += length;
+}
+
+int
+safe_snprintf (char *str,
+ size_t len,
+ const char *format,
+ ...)
+{
+ struct sprintf_ctx cx;
+ int num_args;
+ va_list va;
+ const char **args;
+ int ret;
+
+ cx.data = str;
+ cx.length = 0;
+ cx.alloc = len;
+
+ va_start (va, format);
+ args = valist_to_args (va, &num_args);
+ va_end (va);
+
+ if (args == NULL)
+ return -1;
+
+ if (len)
+ cx.data[0] = '\0';
+
+ ret = safe_printf_cb (snprintf_callback, &cx, format, args, num_args);
+ if (ret < 0)
+ return ret;
+
+ /* Null terminate appropriately */
+ if (len > 0)
+ cx.data[MIN(cx.length, len - 1)] = '\0';
+
+ free (args);
+ return ret;
+}
diff --git a/service/safe-printf.h b/service/safe-printf.h
new file mode 100644
index 0000000..8881b91
--- /dev/null
+++ b/service/safe-printf.h
@@ -0,0 +1,45 @@
+/* realmd -- Realm configuration service
+ *
+ * Copyright 2013 Red Hat Inc
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2 of the licence or (at
+ * your option) any later version.
+ *
+ * See the included COPYING file for more information.
+ *
+ * Author: Stef Walter <stefw@redhat.com>
+ */
+
+#include "config.h"
+
+#ifndef __SAFE_PRINTF_H__
+#define __SAFE_PRINTF_H__
+
+#include <stdlib.h>
+
+#ifndef GNUC_NULL_TERMINATED
+#if __GNUC__ >= 4
+#define GNUC_NULL_TERMINATED __attribute__((__sentinel__))
+#else
+#define GNUC_NULL_TERMINATED
+#endif
+#endif
+
+int safe_asprintf (char **strp,
+ const char *format,
+ ...) GNUC_NULL_TERMINATED;
+
+int safe_snprintf (char *str,
+ size_t len,
+ const char *format,
+ ...) GNUC_NULL_TERMINATED;
+
+int safe_printf_cb (void (* callback) (void *data, const char *piece, size_t len),
+ void *data,
+ const char *format,
+ const char * const args[],
+ int num_args);
+
+#endif /* __SAFE_PRINTF_H__ */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f6d400b..d26c762 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -17,6 +17,7 @@ LDADD = \
TEST_PROGS = \
test-ini-config \
test-sssd-config \
+ test-safe-printf \
test-login-name \
test-samba-ou-format \
test-settings \
@@ -43,6 +44,11 @@ test_sssd_config_SOURCES = \
$(top_srcdir)/service/realm-settings.c \
$(NULL)
+test_safe_printf_SOURCES = \
+ test-safe-printf.c \
+ $(top_srcdir)/service/safe-printf.c \
+ $(NULL)
+
test_login_name_SOURCES = \
test-login-name.c \
$(top_srcdir)/service/realm-login-name.c \
diff --git a/tests/test-safe-printf.c b/tests/test-safe-printf.c
new file mode 100644
index 0000000..fa337ac
--- /dev/null
+++ b/tests/test-safe-printf.c
@@ -0,0 +1,238 @@
+/* realmd -- Realm configuration service
+ *
+ * Copyright 2012 Red Hat Inc
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2 of the licence or (at
+ * your option) any later version.
+ *
+ * See the included COPYING file for more information.
+ *
+ * Author: Stef Walter <stefw@gnome.org>
+ */
+
+#include "config.h"
+
+#include "service/safe-printf.h"
+
+#include <glib.h>
+
+#include <string.h>
+
+typedef struct {
+ const gchar *format;
+ const gchar *args[8];
+ const gchar *result;
+} Fixture;
+
+static void
+callback (void *data,
+ const char *piece,
+ size_t len)
+{
+ g_string_append_len (data, piece, len);
+}
+
+static void
+test_safe_printf (gconstpointer user_data)
+{
+ const Fixture *fixture = user_data;
+ GString *out;
+ int num_args;
+ int ret;
+
+ for (num_args = 0; fixture->args[num_args] != NULL; )
+ num_args++;
+
+ out = g_string_new ("");
+ ret = safe_printf_cb (callback, out, fixture->format, (const gchar **)fixture->args, num_args);
+ if (fixture->result) {
+ g_assert_cmpint (ret, >=, 0);
+ g_assert_cmpstr (out->str, ==, fixture->result);
+ g_assert_cmpint (ret, ==, out->len);
+ } else {
+ g_assert_cmpint (ret, <, 0);
+ }
+
+ g_string_free (out, TRUE);
+}
+
+static const Fixture fixtures[] = {
+ {
+ /* Just a bog standard string */
+ "%s", { "blah", NULL, },
+ "blah"
+ },
+ {
+ /* Empty to print */
+ "%s", { "", NULL, },
+ ""
+ },
+ {
+ /* Nothing to print */
+ "", { "blah", NULL, },
+ ""
+ },
+ {
+ /* Width right aligned */
+ "%8s", { "blah", NULL, },
+ " blah"
+ },
+ {
+ /* Width left aligned */
+ "whoop %-8s doo", { "dee", NULL, },
+ "whoop dee doo"
+ },
+ {
+ /* Width space aligned (ignored) */
+ "whoop % 8s doo", { "dee", NULL, },
+ "whoop dee doo"
+ },
+ {
+ /* Width left space aligned (ignored) */
+ "whoop % -8s doo", { "dee", NULL, },
+ "whoop dee doo"
+ },
+ {
+ /* Precision 1 digit */
+ "whoop %.3s doo", { "deedle-dee", NULL, },
+ "whoop dee doo"
+ },
+ {
+ /* Precision, N digits */
+ "whoop %.10s doo", { "deedle-dee-deedle-do-deedle-dum", NULL, },
+ "whoop deedle-dee doo"
+ },
+ {
+ /* Precision, zero digits */
+ "whoop %.s doo", { "deedle", NULL, },
+ "whoop doo"
+ },
+ {
+ /* Multiple simple arguments */
+ "space %s %s", { "man", "dances", NULL, },
+ "space man dances"
+ },
+ {
+ /* Literal percent */
+ "100%% of space folk dance", { NULL, },
+ "100% of space folk dance"
+ },
+ {
+ /* Multiple simple arguments */
+ "space %2$s %1$s", { "dances", "man", NULL, },
+ "space man dances"
+ },
+ {
+ /* Skipping an argument (not supported by standard printf) */
+ "space %2$s dances", { "dances", "man", NULL, },
+ "space man dances"
+ },
+
+ /* Failures start here */
+
+ {
+ /* Unsupported conversion */
+ "%x", { "blah", NULL, },
+ NULL
+ },
+ {
+ /* Bad positional argument */
+ "space %55$s dances", { "dances", "man", NULL, },
+ NULL
+ },
+ {
+ /* Zero positional argument */
+ "space %0$s dances", { "dances", "man", NULL, },
+ NULL
+ },
+ {
+ /* Too many args used */
+ "%s %s dances", { "space", NULL, },
+ NULL
+ },
+
+};
+
+static void
+test_safe_snprintf (void)
+{
+ char buffer[8];
+ int ret;
+
+ ret = safe_snprintf (buffer, 8, "%s", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 5);
+ g_assert_cmpstr (buffer, ==, "space");
+
+ ret = safe_snprintf (buffer, 8, "", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 0);
+ g_assert_cmpstr (buffer, ==, "");
+
+ ret = safe_snprintf (buffer, 8, "the %s %s dances away", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 25);
+ g_assert_cmpstr (buffer, ==, "the spa");
+
+ ret = safe_snprintf (buffer, 8, "%5$s", NULL);
+ g_assert_cmpint (ret, <, 0);
+}
+
+static void
+test_safe_asprintf (void)
+{
+ char *buffer;
+ int ret;
+
+ ret = safe_asprintf (&buffer, "%s", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 5);
+ g_assert_cmpstr (buffer, ==, "space");
+ free (buffer);
+
+ ret = safe_asprintf (&buffer, "", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 0);
+ g_assert_cmpstr (buffer, ==, "");
+ free (buffer);
+
+ ret = safe_asprintf (&buffer, "the %s %s dances away", "space", "man", NULL);
+ g_assert_cmpint (ret, ==, 25);
+ g_assert_cmpstr (buffer, ==, "the space man dances away");
+ free (buffer);
+
+ ret = safe_asprintf (&buffer, "%1$s %1$s %1$s %1$s %1$s %1$s", "space man", NULL);
+ g_assert_cmpint (ret, ==, 59);
+ g_assert_cmpstr (buffer, ==, "space man space man space man space man space man space man");
+ free (buffer);
+
+ ret = safe_asprintf (&buffer, "%5$s", NULL);
+ g_assert_cmpint (ret, <, 0);
+}
+
+int
+main (int argc,
+ char **argv)
+{
+ gchar *escaped;
+ gchar *name;
+ gint i;
+
+ g_test_init (&argc, &argv, NULL);
+ g_set_prgname ("test-safe-printf");
+
+ for (i = 0; i < G_N_ELEMENTS (fixtures); i++) {
+ if (g_str_equal (fixtures[i].format, ""))
+ escaped = g_strdup ("_empty_");
+ else
+ escaped = g_strdup (fixtures[i].format);
+ g_strdelimit (escaped, " =\\/", '_');
+ name = g_strdup_printf ("/realmd/safe-printf/%s", escaped);
+ g_free (escaped);
+
+ g_test_add_data_func (name, fixtures + i, test_safe_printf);
+ g_free (name);
+ }
+
+ g_test_add_func ("/realmd/safe-snprintf", test_safe_snprintf);
+ g_test_add_func ("/realmd/safe-asprintf", test_safe_asprintf);
+
+ return g_test_run ();
+}
--
1.8.4.2