[PATCHES] MSZIP support for cabinet files

Aurélien Aptel aaptel at suse.com
Wed Jul 19 14:08:19 UTC 2017

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?

> ...
> 			Missing talloc return check.

Good catch.

v4 attached, with corrections and couple of added checks for paranoia,
let me know what you think.

Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mszip-v4.patchset
Type: text/x-patch
Size: 48341 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170719/371284d9/mszip-v4.bin>

More information about the samba-technical mailing list