[PATCH] More ldb/tdb tests.

Jeremy Allison jra at samba.org
Mon Nov 9 19:03:32 UTC 2015


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 :-).

Cheers,

Jeremy.



More information about the samba-technical mailing list