[SSSD] [PATCH] SSH: Ensure that cmd_ctx->name will not be NULL.

Lukas Slebodnik lslebodn at redhat.com
Wed Jul 31 12:16:46 UTC 2013


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
-------------- next part --------------
>From 45fc6c99d692c6bb7b9f0b3e0f2347ccdd0ab990 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at 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



More information about the sssd-devel mailing list