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