Bug 10848 - SMB2 client queryinfo return length checks are incorrect.
SMB2 client queryinfo return length checks are incorrect.
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-01 21:19 UTC by Jeremy Allison
Modified: 2014-10-13 19:10 UTC (History)
0 users

See Also:


Attachments
git-am fix for 4.1.next, 4.0.next. (1.21 KB, patch)
2014-10-02 18:05 UTC, Jeremy Allison
metze: review+
ddiss: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2014-10-01 21:19:34 UTC
Found by Gordon Ross (from samba-technical):

Can anyone explain why this padding check is here?

libcli/smb/smb2cli_query_info.c : 157

if (output_buffer_length < dyn_len) {
    tevent_req_nterror(
        req, NT_STATUS_INVALID_NETWORK_RESPONSE);
    return;
}

That's demanding the that query response data is padded out to
fill the (padded out) length of the SMB2 response.
As far as I can tell, the spec. does not require that,
and the Samba client appears to be the only one
we've run across that insists on this padding.

To clarify, we pad the (outer) SMB2 response to 8 bytes as
required by MS-SMB2, but we don't currently pad out the
query info response data contained therein.
Did I miss something in one of the specs?
Comment 1 Jeremy Allison 2014-10-02 18:05:48 UTC
Created attachment 10322 [details]
git-am fix for 4.1.next, 4.0.next.

Patch cherry-picked that went into master.
Jeremy.
Comment 2 David Disseldorp 2014-10-02 18:58:04 UTC
Karolin, please merge. Thanks!
Comment 3 Karolin Seeger 2014-10-09 18:53:58 UTC
Pushed to autobuild-v4-[0|1|2]-test.

Jeremy, please check if it's really appropriate for 4.2 also. Thanks!
Comment 4 Jeremy Allison 2014-10-09 18:55:49 UTC
(In reply to Karolin Seeger from comment #3)
> Pushed to autobuild-v4-[0|1|2]-test.
> 
> Jeremy, please check if it's really appropriate for 4.2 also. Thanks!

Yes, it needs to be in 4.2.
Comment 5 Karolin Seeger 2014-10-13 19:10:28 UTC
(In reply to Jeremy Allison from comment #4)

Thanks!
Comment 6 Karolin Seeger 2014-10-13 19:10:55 UTC
Pushed to all three branches.
Closing out bug report.

Thanks!