[PATCH] nss_wrappe fix comparison between pointer and '\0'

[Andrew Bartlett]

On Thu, 2017-08-17 at 09:27 +1200, Douglas Bagnall via samba-technical
wrote:
> hi Andreas,
> 
> On 16/08/17 21:35, Andreas Schneider wrote:
> > 
> > Hi Gary and Douglas,
> > 
> > I've fixed that last week, but do not have a RB+ for the patch yet.
> > I think 
> > the if-check for strtol should be as in the attached patch.
> > 
> > 
> > Do you agree?
> > 
> 
> Not quite.

G'Day Douglas and Andreas,

Can we make some progress here?

> > 	Andreas
> > 
> > -- Andreas Schneider GPG-ID: CC014E3D Samba Team asn at samba.org
> > www.samba.org
> > 
> > 
> > 0001-nwrap-Fix-check-after-strtol.patch
> > 
> > 
> > From 908ef658674d912cc611978e97984b7278260c33 Mon Sep 17 00:00:00
> > 2001
> > From: Andreas Schneider <asn at samba.org>
> > Date: Wed, 2 Aug 2017 13:32:08 +0200
> > Subject: [PATCH] nwrap: Fix check after strtol()
> > 
> > Signed-off-by: Andreas Schneider <asn at samba.org>
> > ---
> >  src/nss_wrapper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/nss_wrapper.c b/src/nss_wrapper.c
> > index 70a5989..1d7e2d5 100644
> > --- a/src/nss_wrapper.c
> > +++ b/src/nss_wrapper.c
> > @@ -1577,7 +1577,7 @@ static void nwrap_init(void)
> >  	env = getenv("NSS_WRAPPER_MAX_HOSTENTS");
> >  	if (env != NULL) {
> >  		max_hostents_tmp = (size_t)strtol(env, &endptr,
> > 10);
> > -		if (((env != '\0') && (endptr == '\0')) ||
> > +		if ((env == endptr) ||
> >  		    (max_hostents_tmp == 0)) {
> >  			NWRAP_LOG(NWRAP_LOG_DEBUG,
> >  				  "Error parsing
> > NSS_WRAPPER_MAX_HOSTENTS "
> 
> Isn't the (env == endptr) redundant? It matches an empty string, in
> which case strtol will return zero which is caught by the
> (max_hostents_tmp == 0) case.
> 
> It also doesn't catch a case like NSS_WRAPPER_MAX_HOSTENTS="32xyz",
> where strtol will return 32 and the bad value is indicated by
> *endpointer being 'x'.
> 
> It should actually be strtoul, unless NSS_WRAPPER_MAX_HOSTENTS=-1 is
> meant to be shorthand for pseudo-infinite max_hostents.

It would be really nice to get something fixed here, as Gary keeps on
having his version in all the patches he does (as gcc6 won't build
without it), and we are going to accidentally get it in if we are not
careful.

Thanks,

Andrew Bartlett

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