dom_sid_parse_length (was: Re: [PATCH] Cleanup patches)

[Andrew Bartlett]

On Tue, 2015-04-28 at 15:32 +0200, Volker Lendecke wrote:
> From a76d79222518d239aa24d62ca205821ea02c14a6 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 2 Jan 2015 11:02:45 +0100
> Subject: [PATCH 2/3] lib: Simplify dom_sid_parse_length
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  libcli/security/dom_sid.c |   11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
> index 836979e..2910434 100644
> --- a/libcli/security/dom_sid.c
> +++ b/libcli/security/dom_sid.c
> @@ -243,14 +243,9 @@ struct dom_sid *dom_sid_parse_talloc(TALLOC_CTX *mem_ctx, const char *sidstr)
>  */
>  struct dom_sid *dom_sid_parse_length(TALLOC_CTX *mem_ctx, const DATA_BLOB *sid)
>  {
> -       struct dom_sid *ret;
> -       char *p = talloc_strndup(mem_ctx, (char *)sid->data, sid->length);
> -       if (!p) {
> -               return NULL;
> -       }
> -       ret = dom_sid_parse_talloc(mem_ctx, p);
> -       talloc_free(p);
> -       return ret;
> +       char p[sid->length+1];
> +       memcpy(p, sid->data, sizeof(p));
> +       return dom_sid_parse_talloc(mem_ctx, p);
>  }

Can you please explain to me how this works?  

As I see it, we now read 1 byte beyond the DATA_BLOB length, copy that,
and then presume this uninitialised value is a NULL terminator for
dom_sid_parse_talloc(), but without explicitly setting it.

What am I missing?

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba