On Tue, 2011-11-15 at 17:40 +0100, Jakub Hrozek wrote:
> On Mon, Nov 14, 2011 at 11:35:04AM -0500, Stephen Gallagher wrote:
> > Fixes https://fedorahosted.org/sssd/ticket/1088
> > 
> 
> The approach prevents the dbus crash, but I think you also need to check
> the name during nss_cmd_initgroups()

Thanks for catching that. Fixed.

I also attached a test program I used to verify the fixes. When this
program is run (link it against -lpam and -lpam_misc), it would cause
the sssd_nss or sssd_pam processes to crash. I just commented out all
but one at a time to test each of the processes.
From caa8ee868006dedde6b2afcdb55eaabaee4a4ae3 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Mon, 14 Nov 2011 11:31:37 -0500
Subject: [PATCH] RESPONDER: Ensure that all input strings are valid UTF-8

---
 Makefile.am                             |    6 ++++--
 src/external/libunistring.m4            |    5 +++++
 src/responder/common/responder.h        |    2 ++
 src/responder/common/responder_common.c |    9 +++++++++
 src/responder/nss/nsssrv_cmd.c          |   21 +++++++++++++++++++++
 src/responder/nss/nsssrv_netgroup.c     |    7 +++++++
 src/responder/pam/pamsrv_cmd.c          |    5 +++++
 7 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 58c7928254d6faab3404d1e799a78085e913567e..a423ace3907446fdd1c8e0baca6ffc165ca1d26b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -412,7 +412,8 @@ sssd_nss_SOURCES = \
 sssd_nss_LDADD = \
     $(TDB_LIBS) \
     $(SSSD_LIBS) \
-    libsss_util.la
+    libsss_util.la \
+    -lunistring
 
 sssd_pam_SOURCES = \
     src/responder/pam/pam_LOCAL_domain.c \
@@ -423,7 +424,8 @@ sssd_pam_SOURCES = \
 sssd_pam_LDADD = \
     $(TDB_LIBS) \
     $(SSSD_LIBS) \
-    libsss_util.la
+    libsss_util.la \
+    -lunistring
 
 sssd_be_SOURCES = \
     src/providers/data_provider_be.c \
diff --git a/src/external/libunistring.m4 b/src/external/libunistring.m4
index 69c54fe3faf4bffea120cb30e1d3ed73a2a104c8..18ea3e62a727032a86d248318b5a2a19011ee8ac 100644
--- a/src/external/libunistring.m4
+++ b/src/external/libunistring.m4
@@ -6,4 +6,9 @@ AC_CHECK_HEADERS(unistr.h,
 AC_CHECK_HEADERS(unicase.h,
     [AC_CHECK_LIB([unistring], [u8_casecmp], [ UNISTRING_LIBS="-lunistring" ], [AC_MSG_ERROR([No usable libunistring library found])])],
     [AC_MSG_ERROR([libunistring header files are not installed])]
+)
+
+AC_CHECK_HEADERS(unistr.h,
+    [AC_CHECK_LIB([unistring], [u8_check], [ UNISTRING_LIBS="-lunistring" ], [AC_MSG_ERROR([No usable libunistring library found])])],
+    [AC_MSG_ERROR([libunistring header files are not installed])]
 )
\ No newline at end of file
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index 321cedda8f495e38e19fe07b60271d54915316b7..1b39fdd5b6854e05acdf5b7be2c63a2c8381f3c3 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -174,4 +174,6 @@ int sss_dp_send_acct_req(struct resp_ctx *rctx, TALLOC_CTX *callback_memctx,
 int responder_logrotate(DBusMessage *message,
                         struct sbus_connection *conn);
 
+bool sss_utf8_check(const uint8_t *s, size_t n);
+
 #endif /* __SSS_RESPONDER_H__ */
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 719f2464733faa0498be59226eeb39f5736d0d26..f97ec06fd2d93d9568607c00bb3399792eec397a 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -32,6 +32,7 @@
 #include <sys/time.h>
 #include <errno.h>
 #include <popt.h>
+#include <unistr.h>
 #include "util/util.h"
 #include "db/sysdb.h"
 #include "confdb/confdb.h"
@@ -627,3 +628,11 @@ int responder_logrotate(DBusMessage *message,
 
     return monitor_common_pong(message, conn);
 }
+
+bool sss_utf8_check(const uint8_t *s, size_t n)
+{
+    if (u8_check(s, n) == NULL) {
+        return true;
+    }
+    return false;
+}
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 8f4cb4401defaf9f51d95209533e78f13dbf58d6..a37bd76641b04b004473464bf830b14f180f63a4 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -873,6 +873,13 @@ static int nss_cmd_getpwnam(struct cli_ctx *cctx)
         ret = EINVAL;
         goto done;
     }
+
+    /* If the body isn't valid UTF-8, fail */
+    if (!sss_utf8_check(body, blen)) {
+        ret = EINVAL;
+        goto done;
+    }
+
     rawname = (const char *)body;
 
     domname = NULL;
@@ -2140,6 +2147,13 @@ static int nss_cmd_getgrnam(struct cli_ctx *cctx)
         ret = EINVAL;
         goto done;
     }
+
+    /* If the body isn't valid UTF-8, fail */
+    if (!sss_utf8_check(body, blen)) {
+        ret = EINVAL;
+        goto done;
+    }
+
     rawname = (const char *)body;
 
     domname = NULL;
@@ -3180,6 +3194,13 @@ static int nss_cmd_initgroups(struct cli_ctx *cctx)
         ret = EINVAL;
         goto done;
     }
+
+    /* If the body isn't valid UTF-8, fail */
+    if (!sss_utf8_check(body, blen)) {
+        ret = EINVAL;
+        goto done;
+    }
+
     rawname = (const char *)body;
 
     domname = NULL;
diff --git a/src/responder/nss/nsssrv_netgroup.c b/src/responder/nss/nsssrv_netgroup.c
index cd0ba723ad52ed0abeabd3f60ac1696131743458..7d5665d41788c26cdb71538af4bbbd9f31db1614 100644
--- a/src/responder/nss/nsssrv_netgroup.c
+++ b/src/responder/nss/nsssrv_netgroup.c
@@ -113,6 +113,13 @@ int nss_cmd_setnetgrent(struct cli_ctx *client)
         ret = EINVAL;
         goto done;
     }
+
+    /* If the body isn't valid UTF-8, fail */
+    if (!sss_utf8_check(body, blen)) {
+        ret = EINVAL;
+        goto done;
+    }
+
     rawname = (const char *)body;
 
     req = setnetgrent_send(cmdctx, rawname, cmdctx);
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 1d2a2a58e9780cc4d0b9ed5ee9d1c87663065dcc..18ba3fdf3fd1481846f7e30d56b5b227951fdc86 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -70,6 +70,11 @@ static int extract_string(char **var, size_t size, uint8_t *body, size_t blen,
 
     if (str[size-1]!='\0') return EINVAL;
 
+    /* If the string isn't valid UTF-8, fail */
+    if (!sss_utf8_check(str, size)) {
+        return EINVAL;
+    }
+
     *c += size;
 
     *var = (char *) str;
-- 
1.7.7.1

#include <pwd.h>
#include <stdio.h>
#include <security/pam_appl.h>
#include <security/pam_misc.h>


static struct pam_conv conv = {
    misc_conv,
    NULL
};

int main(int argc, char **argv)
{
    pam_handle_t *pamh;
    int ret;
    const char *name = "ad\351la\357d";

    getpwnam(name);
    getgrnam(name);
    initgroups(name, 0);

    pam_start("sss_test", name, &conv, &pamh);
    ret = pam_authenticate(pamh, 0);
    pam_end(pamh, ret);
    return 0;
}

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to