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

Pavel Březina pbrezina at redhat.com
Wed Jul 31 08:57:36 UTC 2013


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) {
         ...
     }
}




More information about the sssd-devel mailing list