[PATCH v2 2/4] wbclient: fix conversion logic in wbcSidToStringBuf

[Jeff Layton]

On Tue, 30 Jul 2013 16:24:01 -0400
Jeff Layton <jlayton at samba.org> wrote:

> Might as well fix it to handle large authority values properly. Also
> correct some of the formatting.
> 
> Signed-off-by: Jeff Layton <jlayton at samba.org>
> ---
>  nsswitch/libwbclient/wbc_sid.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c
> index 99091c9..47a0be0 100644
> --- a/nsswitch/libwbclient/wbc_sid.c
> +++ b/nsswitch/libwbclient/wbc_sid.c
> @@ -32,7 +32,7 @@
>   * result if it was long enough. */
>  int wbcSidToStringBuf(const struct wbcDomainSid *sid, char *buf, int buflen)
>  {
> -	uint32_t id_auth;
> +	uint64_t id_auth;
>  	int i, ofs;
>  
>  	if (!sid) {
> @@ -40,22 +40,25 @@ int wbcSidToStringBuf(const struct wbcDomainSid *sid, char *buf, int buflen)
>  		return 10;	/* strlen("(NULL SID)") */
>  	}
>  
> -	/*
> -	 * BIG NOTE: this function only does SIDS where the identauth is not
> -	 * >= ^32 in a range of 2^48.
> -	 */
> -
> -	id_auth = sid->id_auth[5] +
> -		(sid->id_auth[4] << 8) +
> -		(sid->id_auth[3] << 16) +
> -		(sid->id_auth[2] << 24);
> +	id_auth = (uint64_t)sid->id_auth[5] +
> +		((uint64_t)sid->id_auth[4] << 8) +
> +		((uint64_t)sid->id_auth[3] << 16) +
> +		((uint64_t)sid->id_auth[2] << 24) +
> +		((uint64_t)sid->id_auth[1] << 32) +
> +		((uint64_t)sid->id_auth[0] << 40);
>  
> -	ofs = snprintf(buf, buflen, "S-%u-%lu",
> -		       (unsigned int)sid->sid_rev_num, (unsigned long)id_auth);
> +	ofs = snprintf(buf, buflen, "S-%hhu-", (unsigned char)sid->sid_rev_num);
> +	if (id_auth >= UINT32_MAX) {
> +		ofs += snprintf(buf + ofs, MAX(buflen - ofs, 0), "-0x%llx",
> +				(unsigned long long)id_auth);
> +	} else {
> +		ofs += snprintf(buf + ofs, MAX(buflen - ofs, 0), "-%llu",
> +				(unsigned long long)id_auth);
> +	}

<facepalm>

The above code will add an extra '-' between the revision and id_auth.
Please don't merge this yet. I'll respin with a v3 set (and try to
include some tests for this like Andrew requested.

>  
>  	for (i = 0; i < sid->num_auths; i++) {
> -		ofs += snprintf(buf + ofs, MAX(buflen - ofs, 0), "-%lu",
> -				(unsigned long)sid->sub_auths[i]);
> +		ofs += snprintf(buf + ofs, MAX(buflen - ofs, 0), "-%u",
> +				(unsigned int)sid->sub_auths[i]);
>  	}
>  	return ofs;
>  }


-- 
Jeff Layton <jlayton at samba.org>