Quota patches

Jeremy Allison jra at samba.org
Thu Jul 26 22:48:19 UTC 2018


On Thu, Jul 26, 2018 at 04:31:37PM +0100, Noel Power via samba-technical wrote:
> Hi Jeremy,
> 
> Please see attached v2 of the patch and also inline comments
> That actually is already in place, this is in a while loop so the test
> is actually at the end of the loop (after offset is pulled off the
> wire). Additionally the test is against sidlistlength because we are
> sure that sidlistlength is at least less than the size of the sid_buf.
> Note: the caller of 'extract_sids_from_buf' has checked that, but... you
> are correct, this function should check it itself so I have added the
> test to make sureĀ  sidlistlength doesn't exceed the size of the buffer
> passed in, all subsequent tests are against sidlistlength (which in
> could be less than size the sid_buf passed in)

Actually, I've gone through this really carefully now,
and this code:

               ZERO_STRUCT(info);
                info.sid_length = ndr_size_dom_sid(&tmp_list->quotas->sid, 0);

                dsize = sizeof(info.next_entry_offset)
                        + sizeof(info.sid_length)
                        + sizeof(info.change_time)
                        + sizeof(info.quota_used)
                        + sizeof(info.quota_threshold)
                        + sizeof(info.quota_limit);

                if (dsize + info.sid_length < dsize) {
                        /* Integer wrap */
                        DBG_ERR("integer wrap dsize %d sid_length %d\n",
                                dsize, info.sid_length);
                        return NT_STATUS_INVALID_PARAMETER;
                }

simply can't wrap, whatever the client or server
puts in tmp_list->quotas->sid.

ndr_size_dom_sid() cannot return an 32-bit integer
wrapped number or even close to that, plus the
sizeof(<type>) calls also will not return anything
in danger of wrap.

As you're using ndr_push_XXX() auto-generated
functions that do dynamic allocation, all this
code can be dropped (sorry if it's my fault
you added it in the first place, but I really
needed to understand this before I could safely
review it).

More once I've fixed up the patchset and I'll
send it back to you for review.

Jeremy.



More information about the samba-technical mailing list