[PATCHES] MSZIP support for cabinet files

Jeremy Allison jra at samba.org
Wed Jul 19 19:12:18 UTC 2017


On Wed, Jul 19, 2017 at 04:08:19PM +0200, Aurélien Aptel wrote:
> Jeremy Allison <jra at samba.org> writes:
> > In ndr_push_folder_cfdata()
> >
> > +                       size_t old_off = ndr->offset;
> > +                       /* just copy the data */
> >
> > Here you push the data.
> >
> > +                       NDR_CHECK(ndr_push_bytes(ndr, r->ab.data, r->ab.length));
> >
> > Then you do the overflow check. That's backwards.
> 
> > ...
> 
> > When you do a ndr_push_bytes(ndr, data, length) you *KNOW*
> > it's going to advance ndr->offset by length bytes.
> >
> > So in that case do the wrap checks *before* the push.
> 
> I guess there's a misunderstanding, that's literally what you asked me
> to do previously:
> 
> > What if they don't match ? The other case statement uses the
> > offset from struct ndr_push * after the push has been done.
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Can we do the same in the uncompressed case too ? Or maybe
> > abort if r->cbUncomp != r->ab.length in the uncompressed
> > case (as they should be the same).
> 
> I also *know* NDR already checks for overflows with the *_NEED_BYTES().
> 
> > Also, look at all the other wrap checks in Samba.
> > They should always look like:
> >
> > if (a + b < a) {
> > 	error
> > }
> 
> I think I'll use an extra NEED_BYTES() call here which I think is even
> less error prone and was made just for this purpose. Sounds good?

LGTM !!!!! Reviewed-by: Jeremy Allison <jra at samba.org>

Pushed. Thanks a *LOT* for your patience on this.

Jeremy.



More information about the samba-technical mailing list