On (31/07/13 10:57), Pavel Březina wrote: >On 07/30/2013 02:42 PM, Lukas Slebodnik wrote: >>ehlo, >> >>Ensure that cmd_ctx->name will not be NULL. >> >>If cmd_ctx->name was not initialized by sss_parse_name >>then copy of name will be used. >> >>https://fedorahosted.org/sssd/ticket/1970 >>Coverity ID: 11647 >> >>Patch is attached. >> >>LS > >Nack. > >> if (cmd_ctx->name == NULL) { >> cmd_ctx->name = talloc_strdup(cmd_ctx, name); >> if (!cmd_ctx->name) return ENOMEM; >> } >> >> if (cmd_ctx->is_user && cmd_ctx->domname == NULL) { >> DEBUG(SSSDBG_TRACE_FUNC, >> ("Parsing name [%s][%s]\n", name, domain ? domain : "<ALL>")); >> > >You need to talloc_zfree(cmd_ctx->name) before reassigning it. > >> ret = sss_parse_name_for_domains(cmd_ctx, cctx->rctx->domains, >> domain, name, >> &cmd_ctx->domname, >> &cmd_ctx->name); >> if (ret != EOK) { >> DEBUG(SSSDBG_OP_FAILURE, >> ("Invalid name received [%s]\n", name)); >> return ENOENT; >> } >> } else if (cmd_ctx->domname == NULL) { >> if (domain != NULL) { >> cmd_ctx->domname = talloc_strdup(cmd_ctx, domain); >> if (!cmd_ctx->domname) return ENOMEM; >> } >> } >> >> if (alias != NULL && strcmp(cmd_ctx->name, alias) != 0) { >> cmd_ctx->alias = talloc_strdup(cmd_ctx, alias); >> if (!cmd_ctx->alias) return ENOMEM; >> } > >However, I think it would be read better this way: > >if (cmd_ctx->is_user && cmd_ctx->domname == NULL) { > ... >} else { > if (cmd_ctx->name == NULL) { > ... > } > > if (cmd_ctx->domname == NULL && domain != NULL) { > ... > } >}
Thank you for review, It is a nice catch. I should not write patch anymore in the meeting time. Updated patch is attached. LS
>From 45fc6c99d692c6bb7b9f0b3e0f2347ccdd0ab990 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Wed, 31 Jul 2013 14:12:48 +0200 Subject: [PATCH] SSH: Ensure that cmd_ctx->name will not be NULL. If cmd_ctx->name was not initialized by sss_parse_name then copy of name will be used. https://fedorahosted.org/sssd/ticket/1970 Coverity ID: 11647 --- src/responder/ssh/sshsrv_cmd.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c index 76c3643506ffa1155a95750f96ee84305b1b317d..efa4513475a4257eaf82b136c1c2a36817721f4a 100644 --- a/src/responder/ssh/sshsrv_cmd.c +++ b/src/responder/ssh/sshsrv_cmd.c @@ -765,11 +765,13 @@ ssh_cmd_parse_request(struct ssh_cmd_ctx *cmd_ctx) ("Invalid name received [%s]\n", name)); return ENOENT; } - } else if (cmd_ctx->name == NULL && cmd_ctx->domname == NULL) { - cmd_ctx->name = talloc_strdup(cmd_ctx, name); - if (!cmd_ctx->name) return ENOMEM; + } else { + if (cmd_ctx->name == NULL) { + cmd_ctx->name = talloc_strdup(cmd_ctx, name); + if (!cmd_ctx->name) return ENOMEM; + } - if (domain != NULL) { + if (cmd_ctx->domname == NULL && domain != NULL) { cmd_ctx->domname = talloc_strdup(cmd_ctx, domain); if (!cmd_ctx->domname) return ENOMEM; } -- 1.8.3.1
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel