[SSSD] [PATCH] NSS: fix a use-after-free issue
Sumit Bose
sbose at redhat.com
Mon Nov 2 09:05:47 UTC 2015
On Mon, Nov 02, 2015 at 09:42:51AM +0100, Lukas Slebodnik wrote:
> On (30/10/15 17:35), Sumit Bose wrote:
> >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
> >
>
> >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");
> it would be good to log error code here.
> > goto done;
>
> Was "use-after-free" caused only by logging sid or was it used elsewhere?
ah, sorry. Yes, I made two changes here, replacing the variable in the
debug message and returning immediately and no go to done. The
use-after-free was in the debug message. cmdctx is not touched in the
following code path anymore but since we are all set I think it is
safer to just return here.
>
> Could you append valgrind error to commit mesage?
I found the issue via a core dump. Do you prefer the valgrind message or
the first steps of a stack trace?
bye,
Sumit
More information about the sssd-devel
mailing list