Re: [SSSD] [PATCH] NSS: Fix use after free
On Wed, Aug 19, 2015 at 11:15:16PM +0200, Jakub Hrozek wrote: > On Thu, Aug 13, 2015 at 07:41:02AM +0200, Lukas Slebodnik wrote: > > On (12/08/15 14:17), Jakub Hrozek wrote: > > >On Mon, Aug 10, 2015 at 06:38:29AM +0200, Lukas Slebodnik wrote: > > >> ehlo, > > >> > > >> Use after free can happed if there are two domains and user is not found > > >> in the first one. > > >> > > >> LS > > > > > >Would it be possible to write a testcase in the NSS responder test? > > It requires multi domain setup. > > So I created different test. > > My intention was to cover most test cases and not just initgroups, > > But attached ins a POC patch which prove there is a use after free. > > make check passes; you need to test with valgrind. > > libtool --mode=execute valgrind -v ./nss-srv-multi-tests > > > > Would you prefer to use current version of patch and add othter test cases > > later? (it will take some time) or current version is enought for fix? > > Ideally I think we should have only one NSS responder test, otherwise we > would end up adding some testcases to one test and not the other...but I > haven't tried, so I don't know how easy or hard that is. > > ACK to your crash patch, I'll push it and apply to downstream. Sorry, I forgot to send push-mail: b9901fe3d6cfe05cd75a2440c0f9c7985aea36c6 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] NSS: Fix use after free
On Thu, Aug 13, 2015 at 07:41:02AM +0200, Lukas Slebodnik wrote: On (12/08/15 14:17), Jakub Hrozek wrote: On Mon, Aug 10, 2015 at 06:38:29AM +0200, Lukas Slebodnik wrote: ehlo, Use after free can happed if there are two domains and user is not found in the first one. LS Would it be possible to write a testcase in the NSS responder test? It requires multi domain setup. So I created different test. My intention was to cover most test cases and not just initgroups, But attached ins a POC patch which prove there is a use after free. make check passes; you need to test with valgrind. libtool --mode=execute valgrind -v ./nss-srv-multi-tests Would you prefer to use current version of patch and add othter test cases later? (it will take some time) or current version is enought for fix? Ideally I think we should have only one NSS responder test, otherwise we would end up adding some testcases to one test and not the other...but I haven't tried, so I don't know how easy or hard that is. ACK to your crash patch, I'll push it and apply to downstream. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] NSS: Fix use after free
On (12/08/15 14:17), Jakub Hrozek wrote:
On Mon, Aug 10, 2015 at 06:38:29AM +0200, Lukas Slebodnik wrote:
ehlo,
Use after free can happed if there are two domains and user is not found
in the first one.
LS
Would it be possible to write a testcase in the NSS responder test?
It requires multi domain setup.
So I created different test.
My intention was to cover most test cases and not just initgroups,
But attached ins a POC patch which prove there is a use after free.
make check passes; you need to test with valgrind.
libtool --mode=execute valgrind -v ./nss-srv-multi-tests
Would you prefer to use current version of patch and add othter test cases
later? (it will take some time) or current version is enought for fix?
LS
From a0043e064cf60a2df53ff544fc9c88eb85af6853 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Mon, 10 Aug 2015 12:43:22 +0200
Subject: [PATCH] test_nss_srv_multi: Subject
Explanation
Resolves:
https://fedorahosted.org/sssd/ticket/
---
Makefile.am | 28 +++
src/tests/cmocka/test_nss_srv_multi.c | 437 ++
2 files changed, 465 insertions(+)
create mode 100644 src/tests/cmocka/test_nss_srv_multi.c
diff --git a/Makefile.am b/Makefile.am
index
8b64317d6dce9a1ee8614916395b9afd9f11f382..01e5c6c9c1af21d60a0d4922b327dada94f87704
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -201,6 +201,7 @@ endif # HAVE_CHECK
if HAVE_CMOCKA
non_interactive_cmocka_based_tests = \
nss-srv-tests \
+nss-srv-multi-tests \
test-find-uid \
test-io \
test-negcache \
@@ -1850,6 +1851,33 @@ TEST_MOCK_PROVIDER_OBJ = \
src/tests/cmocka/common_mock_sdap.c \
src/tests/cmocka/common_mock_sysdb_objects.c
+EXTRA_nss_srv_multi_tests_DEPENDENCIES = \
+ $(ldblib_LTLIBRARIES)
+nss_srv_multi_tests_SOURCES = \
+ $(TEST_MOCK_RESP_OBJ) \
+ src/tests/cmocka/test_nss_srv_multi.c \
+ src/responder/nss/nsssrv_cmd.c \
+ src/responder/nss/nsssrv_netgroup.c \
+ src/responder/nss/nsssrv_services.c \
+ src/responder/nss/nsssrv_mmap_cache.c
+nss_srv_multi_tests_CFLAGS = \
+$(AM_CFLAGS)
+nss_srv_multi_tests_LDFLAGS = \
+-Wl,-wrap,sss_dp_get_account_send \
+-Wl,-wrap,sss_ncache_check_user \
+-Wl,-wrap,sss_ncache_check_uid \
+-Wl,-wrap,sss_ncache_check_sid \
+-Wl,-wrap,sss_packet_get_body \
+-Wl,-wrap,sss_packet_get_cmd \
+-Wl,-wrap,sss_cmd_send_empty \
+-Wl,-wrap,sss_cmd_done
+nss_srv_multi_tests_LDADD = \
+$(CMOCKA_LIBS) \
+$(SSSD_LIBS) \
+$(SSSD_INTERNAL_LTLIBS) \
+libsss_test_common.la \
+libsss_idmap.la
+
EXTRA_nss_srv_tests_DEPENDENCIES = \
$(ldblib_LTLIBRARIES)
nss_srv_tests_SOURCES = \
diff --git a/src/tests/cmocka/test_nss_srv_multi.c
b/src/tests/cmocka/test_nss_srv_multi.c
new file mode 100644
index
..6320255978b3f8056e0e63d502b9617477d4b042
--- /dev/null
+++ b/src/tests/cmocka/test_nss_srv_multi.c
@@ -0,0 +1,437 @@
+/*
+Authors:
+Jakub Hrozek jhro...@redhat.com
+
+Copyright (C) 2013 Red Hat
+
+SSSD tests: NSS responder tests
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+
+#include talloc.h
+#include tevent.h
+#include errno.h
+#include popt.h
+
+#include tests/cmocka/common_mock.h
+#include tests/cmocka/common_mock_resp.h
+#include responder/common/negcache.h
+#include responder/nss/nsssrv.h
+#include responder/nss/nsssrv_private.h
+#include sss_client/idmap/sss_nss_idmap.h
+#include util/util_sss_idmap.h
+#include db/sysdb_private.h /* new_subdomain() */
+
+#define TESTS_PATH tests_nss_multi
+#define TEST_CONF_DB test_nss_multi_conf.ldb
+#define TEST_DOM_NAME nss_test
+#define TEST_DOM_NAME1 nss_test_a
+#define TEST_DOM_NAME2 nss_test_b
+#define TEST_SUBDOM_NAME test.subdomain
+#define TEST_ID_PROVIDER ldap
+#define TEST_DOM_SID S-1-5-21-444379608-1639770488-2995963434
+
+static const char *domains[] = { TEST_DOM_NAME1, TEST_DOM_NAME2, NULL };
+
+struct nss_test_ctx {
+struct sss_test_ctx *tctx;
+struct sss_domain_info *subdom;
+
+struct resp_ctx *rctx;
+struct cli_ctx *cctx;
+struct sss_cmd_table *nss_cmds;
+struct nss_ctx *nctx;
+
+int ncache_hits;
+};
+
+const char *global_extra_attrs[] = { phone, mobile, NULL };
+
+struct nss_test_ctx *nss_test_ctx;
+
+/* Mock NSS structure */
+struct
Re: [SSSD] [PATCH] NSS: Fix use after free
On Mon, Aug 10, 2015 at 06:38:29AM +0200, Lukas Slebodnik wrote: ehlo, Use after free can happed if there are two domains and user is not found in the first one. LS Would it be possible to write a testcase in the NSS responder test? ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] NSS: Fix use after free
ehlo,
Use after free can happed if there are two domains and user is not found
in the first one.
LS
From cbd388699c061ad644ead6c21e38dfb9bc0ca30c Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 7 Aug 2015 14:29:45 +0200
Subject: [PATCH] NSS: Fix use after free
It can happed if there are two domains and user is not found
in the first one.
==29279== Invalid read of size 1
==29279==at 0x4C2CBA2: strlen (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29279==by 0x89A7AC4: talloc_strdup (in /usr/lib64/libtalloc.so.2.1.2)
==29279==by 0x11668A: nss_cmd_initgroups_search (nsssrv_cmd.c:4191)
==29279==by 0x118B27: nss_cmd_getby_dp_callback (nsssrv_cmd.c:1208)
==29279==by 0x10F2B4: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:759)
==29279==by 0x126AFB: sss_dp_internal_get_done (responder_dp.c:802)
==29279==by 0x56EA861: ??? (in /usr/lib64/libdbus-1.so.3.7.4)
==29279==by 0x56EDB50: dbus_connection_dispatch (in
/usr/lib64/libdbus-1.so.3.7.4)
==29279==by 0x50721E1: sbus_dispatch (sssd_dbus_connection.c:96)
==29279==by 0x879B22E: tevent_common_loop_timer_delay (tevent_timed.c:341)
==29279==by 0x879C239: epoll_event_loop_once (tevent_epoll.c:911)
==29279==by 0x879A936: std_event_loop_once (tevent_standard.c:114)
==29279== Address 0xbbad240 is 96 bytes inside a block of size 106 free'd
==29279==at 0x4C2AD17: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29279==by 0x89A46E3: _talloc_free (in /usr/lib64/libtalloc.so.2.1.2)
==29279==by 0x116679: nss_cmd_initgroups_search (nsssrv_cmd.c:4190)
==29279==by 0x118B27: nss_cmd_getby_dp_callback (nsssrv_cmd.c:1208)
==29279==by 0x10F2B4: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:759)
==29279==by 0x126AFB: sss_dp_internal_get_done (responder_dp.c:802)
==29279==by 0x56EA861: ??? (in /usr/lib64/libdbus-1.so.3.7.4)
==29279==by 0x56EDB50: dbus_connection_dispatch (in
/usr/lib64/libdbus-1.so.3.7.4)
==29279==by 0x50721E1: sbus_dispatch (sssd_dbus_connection.c:96)
==29279==by 0x879B22E: tevent_common_loop_timer_delay (tevent_timed.c:341)
==29279==by 0x879C239: epoll_event_loop_once (tevent_epoll.c:911)
==29279==by 0x879A936: std_event_loop_once (tevent_standard.c:114)
Resolves:
https://fedorahosted.org/sssd/ticket/2749
---
src/responder/nss/nsssrv_cmd.c | 6 +++---
src/responder/nss/nsssrv_private.h | 1 +
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index
e754245eac7200c2f667760540a1d04c9203843d..43cdb135c0933117743a97d6a2524326079bf72c
100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4143,7 +4143,7 @@ static int nss_cmd_initgr_send_reply(struct nss_dom_ctx
*dctx)
}
ret = fill_initgr(cctx-creq-out, dctx-domain, dctx-res, nctx,
- dctx-mc_name, cmdctx-name);
+ dctx-mc_name, cmdctx-normalized_name);
if (ret) {
return ret;
}
@@ -4187,14 +4187,14 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx
*dctx)
/* make sure to update the dctx if we changed domain */
dctx-domain = dom;
-talloc_free(name);
+talloc_zfree(cmdctx-normalized_name);
name = sss_get_cased_name(dctx, cmdctx-name, dom-case_sensitive);
if (!name) return ENOMEM;
name = sss_reverse_replace_space(cmdctx, name,
nctx-rctx-override_space);
/* save name so it can be used in initgr reply */
-cmdctx-name = name;
+cmdctx-normalized_name = name;
if (name == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
sss_reverse_replace_space failed\n);
diff --git a/src/responder/nss/nsssrv_private.h
b/src/responder/nss/nsssrv_private.h
index
e5a2486f1fb9a8de39ec90f802f596b2c2f6af7f..72f7b75604567f9b95937018e54ba2d60b771f9b
100644
--- a/src/responder/nss/nsssrv_private.h
+++ b/src/responder/nss/nsssrv_private.h
@@ -31,6 +31,7 @@ struct nss_cmd_ctx {
struct cli_ctx *cctx;
enum sss_cli_command cmd;
char *name;
+const char *normalized_name;
bool name_is_upn;
uint32_t id;
char *secid;
--
2.5.0
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
