From 9561733a6ce32b5787acc5277e2d45026c5126db Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 30 Oct 2015 16:28:37 +0100 Subject: [PATCH] NSS: fix a use-after-free issue While handling well-known SIDs a debug statement tries to access memory that is already freed. This can be seen with the following output from valgrind. ==17600== Invalid read of size 4 ==17600== at 0x805ACC6: nss_cmd_getbysid (nsssrv_cmd.c:5458) ==17600== by 0x805AF41: nss_cmd_getnamebysid (nsssrv_cmd.c:5509) ==17600== by 0x80662F4: sss_cmd_execute (responder_cmd.c:161) ==17600== by 0x8067015: client_cmd_execute (responder_common.c:249) ==17600== by 0x80671F5: client_recv (responder_common.c:283) ==17600== by 0x806741C: client_fd_handler (responder_common.c:335) ==17600== by 0x45F5112: epoll_event_loop (tevent_epoll.c:728) ==17600== by 0x45F5112: epoll_event_loop_once (tevent_epoll.c:926) ==17600== by 0x45F32EE: std_event_loop_once (tevent_standard.c:114) ==17600== by 0x45EF3BF: _tevent_loop_once (tevent.c:530) ==17600== by 0x45EF5AB: tevent_common_loop_wait (tevent.c:634) ==17600== by 0x45F326E: std_event_loop_wait (tevent_standard.c:140) ==17600== by 0x45EF647: _tevent_loop_wait (tevent.c:653) ==17600== Address 0x4b248a0 is 72 bytes inside a block of size 88 free'd ==17600== at 0x402C26D: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) ==17600== by 0x45FEC9E: _talloc_free_internal (talloc.c:1057) ==17600== by 0x45FEC9E: _talloc_free (talloc.c:1581) ==17600== by 0x8066085: sss_cmd_done (responder_cmd.c:93) ==17600== by 0x805A9B0: nss_check_well_known_sid (nsssrv_cmd.c:5382) ==17600== by 0x805AC86: nss_cmd_getbysid (nsssrv_cmd.c:5455) ==17600== by 0x805AF41: nss_cmd_getnamebysid (nsssrv_cmd.c:5509) ==17600== by 0x80662F4: sss_cmd_execute (responder_cmd.c:161) ==17600== by 0x8067015: client_cmd_execute (responder_common.c:249) ==17600== by 0x80671F5: client_recv (responder_common.c:283) ==17600== by 0x806741C: client_fd_handler (responder_common.c:335) ==17600== by 0x45F5112: epoll_event_loop (tevent_epoll.c:728) ==17600== by 0x45F5112: epoll_event_loop_once (tevent_epoll.c:926) ==17600== by 0x45F32EE: std_event_loop_once (tevent_standard.c:114) ==17600== The patch contains a change to the unit tests which frees the memory in the wrapper for sss_cmd_done() too. This allows to detect this kind of issue in the unit tests as well. --- src/responder/nss/nsssrv_cmd.c | 10 ++++++---- src/tests/cmocka/test_nss_srv.c | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index b8bd6425e2c937ce6008fd6663fe0312ad68f01e..58d20fab11eea6f419031f439083ca5f6d8c61bb 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -5455,11 +5455,13 @@ static int nss_cmd_getbysid(enum sss_cli_command cmd, struct cli_ctx *cctx) ret = nss_check_well_known_sid(cmdctx); if (ret != ENOENT) { if (ret == EOK) { - DEBUG(SSSDBG_TRACE_ALL, "SID [%s] is a Well-Known SID.\n", - cmdctx->secid); - } else { - DEBUG(SSSDBG_OP_FAILURE, "nss_check_well_known_sid failed.\n"); + DEBUG(SSSDBG_TRACE_ALL, "SID [%s] is a Well-Known SID.\n", sid_str); + /* message is already send and cmdctx is freed, + * we can just return */ + return EOK; } + + DEBUG(SSSDBG_OP_FAILURE, "nss_check_well_known_sid failed.\n"); goto done; } diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 6bfbd574a4bb73f932d7ce7e0275769e4c43aa24..f05b55e461d9a058a4894800b2efae3417c8dc01 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -136,6 +136,7 @@ void __wrap_sss_cmd_done(struct cli_ctx *cctx, void *freectx) nss_test_ctx->tctx->error = check_cb(sss_packet_get_status(packet), body, blen); nss_test_ctx->tctx->done = true; + talloc_free(freectx); } enum sss_cli_command __wrap_sss_packet_get_cmd(struct sss_packet *packet) -- 2.1.0