From 05d657339a80405900f8a066e41f08c121755cbf Mon Sep 17 00:00:00 2001 From: Stef Walter 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 #include @@ -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 + */ + +#include "config.h" + +#include "safe-printf.h" + +#include +#include + +#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 + */ + +#include "config.h" + +#ifndef __SAFE_PRINTF_H__ +#define __SAFE_PRINTF_H__ + +#include + +#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 + */ + +#include "config.h" + +#include "service/safe-printf.h" + +#include + +#include + +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