[PATCH] [tldap] check for successful string conversion

[swen]

On Thu, 2019-01-10 at 11:27 +0100, Volker Lendecke via samba-technical
wrote:
> On Thu, Jan 10, 2019 at 11:09:09AM +0100, swen via samba-technical
> wrote:
> > +
> > +	errno = 0;
> >  	result = strtoull(str, NULL, 10);
> > +	if (errno != 0) {
> > +		DBG_DEBUG("Attribute conversion failed (%s)\n",
> > +			  strerror(errno));
> > +		TALLOC_FREE(str);
> > +		return false;
> > +	}
> > +
> 
> The return behaviour of strtoull is pretty weird. Posix says:
> 
> > Upon successful completion, these functions shall return the
> > converted
> > value, if any. If no conversion could be performed, 0 shall be
> > returned [Option Start]  and errno may be set to [EINVAL]. [Option
> > End] If the correct value is outside the range of representable
> > values, {ULONG_MAX} or {ULLONG_MAX} shall be returned and errno set
> > to
> > [ERANGE].
> 
> Assuming that all implementations actually do the [Option] piece, we
> need to check for result 0 or ULLONG_MAX *together* with errno. Just
> checking for errno is not sufficient.
It is sufficient because errno is set to zero before strtoull() is
triggered.
> 
> You might want to take a look at source3/lib/messages_dgm.c:1460ff.
> All the other callers seem not to do this right at the moment.
In fact, this reference is wrong.
You cannot assume, that if the return value is zero and errno == EINVAL
that your conversion failed. The errno might have been be set to EINVAL
from an earlier command and strtoull just "correctly" returned zero.
This way you end up with a false positive.

Cheers Swen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190110/397886ce/signature.sig>