[PATCH] More ldb/tdb tests.

Andrew Bartlett abartlet at samba.org
Fri Nov 13 06:02:42 UTC 2015


On Mon, 2015-11-09 at 11:03 -0800, Jeremy Allison wrote:
> On Sat, Nov 07, 2015 at 09:15:04AM +0100, Stefan Metzmacher wrote:
> > 
> > I don't like integeter wrap checks, they always completely
> > confusing to me,
> > what about:
> > 
> > for (j=0; j < num_values; j++) {
> >     uint32_t len;
> > 
> >     if (remaining < 4) {
> >         return false;
> >     }
> >     len = pull_uint32(p, 0);
> >     p += 4;
> >     remaining -= 4;
> > 
> >     if (remaining < len) {
> >         return false;
> >     }
> >     remaining -= len;
> >     p += len;
> > 
> >     if (remaining < 1) {
> >         return false;
> >     }
> >     remaining -= 1;
> >     p += 1;
> > }
> > 
> > subtraction checks are much easier to understand for me
> > compared to integer wrap checks.
> > 
> > > 	*premaining = remaining;
> > > 	*pp = p;
> > > 	return true;
> > > }
> > > 
> > > Also the code changes inside ldb_unpack_data()
> > > require the same paranoia.
> > > 
> > > These:
> > > 
> > > +                       if (!keep) {
> > > +                               remaining -= len + 1;
> > > +                               p += len + 1;
> > > 
> > > and these:
> > > 
> > > +               element->flags = 0;
> > >                 remaining -= len + 1;
> > >                 p += len + 1;
> > > 
> > > require the range + wrap check paranoia tests.
> > 
> > Same here just handle len and 1 separately.
> 
> Yeah, that works too and is clearer so I'll
> certainly +1 that. So long as we have the checks,
> that's the main thing :-).

Thanks.  I'll have an updated patch to you next week.

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









More information about the samba-technical mailing list