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

+