[realmd/f20] Don't pass user defined strings to printf directly
Stef Walter
stefw at fedoraproject.org
Tue Jan 7 17:13:17 UTC 2014
commit b5a58fab29960908356c1272f2e7bae681c1ab67
Author: Stef Walter <stefw at redhat.com>
Date: Tue Jan 7 16:22:11 2014 +0100
Don't pass user defined strings to printf directly
Resolves: rhbz#1049447
realmd.spec | 8 +-
safe-printf.patch | 792 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 799 insertions(+), 1 deletions(-)
---
diff --git a/realmd.spec b/realmd.spec
index f63c221..78737aa 100644
--- a/realmd.spec
+++ b/realmd.spec
@@ -1,11 +1,12 @@
Name: realmd
Version: 0.14.6
-Release: 2%{?dist}
+Release: 3%{?dist}
Summary: Kerberos realm enrollment service
License: LGPLv2+
URL: http://cgit.freedesktop.org/realmd/realmd/
Source0: http://www.freedesktop.org/software/realmd/releases/realmd-%{version}.tar.gz
Patch1: oddjob-start.patch
+Patch2: safe-printf.patch
BuildRequires: intltool pkgconfig
BuildRequires: gettext-devel
@@ -17,6 +18,7 @@ BuildRequires: krb5-devel
BuildRequires: systemd-devel
BuildRequires: libxslt
BuildRequires: xmlto
+BuildRequires: automake
Requires: authconfig
Requires: oddjob-mkhomedir
@@ -38,6 +40,7 @@ applications that use %{name}.
%prep
%setup -q
%patch1 -p1
+%patch2 -p1
%build
%configure --disable-silent-rules
@@ -72,6 +75,9 @@ make install DESTDIR=%{buildroot}
%doc %{_datadir}/doc/realmd/
%changelog
+* Tue Jan 07 2014 Stef Walter <stefw at redhat.com> - 0.14.6-3
+- Don't pass user defined strings to printf directly [#1049447]
+
* Fri Sep 20 2013 Stef Walter <stefw at redhat.com> - 0.14.6-2
- Start oddjob after joining a domain [#967023]
diff --git a/safe-printf.patch b/safe-printf.patch
new file mode 100644
index 0000000..62b9d4b
--- /dev/null
+++ b/safe-printf.patch
@@ -0,0 +1,792 @@
+From 05d657339a80405900f8a066e41f08c121755cbf Mon Sep 17 00:00:00 2001
+From: Stef Walter <stefw at 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 at 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 at 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 at 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
More information about the scm-commits
mailing list