[SSSD] [PATCH] NSS: fix a use-after-free issue

Sumit Bose sbose at redhat.com
Fri Oct 30 16:35:40 UTC 2015


Hi,

I found this accidentally because I was still running SSSD with
MALLOC_PERTURB_ set which I used some time ago to hunt a glibc issue.

This issue wasn't caught before by the unit-tests because
sss_cmd_done() which frees the context is overwritten in the tests and
so far didn't free the context. Additionally, since the use after free
was in a debug message, the unit tests must be run with debugging enable
to run over it. I think this is also the reason why Coverity or Clang
didn't found this issue before. Does anyone know if it is possible to
tell Converity to assume debug_level is set to 10?

bye,
Sumit

-------------- next part --------------
From 309f59199c49d3d4dc4fa42229f6b2fa1e82e72a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 30 Oct 2015 16:28:37 +0100
Subject: [PATCH] NSS: fix a use-after-free issue

---
 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



More information about the sssd-devel mailing list