[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