Bug 14443 - Out-by-4 error in smbd read reply max_send clamp
Summary: Out-by-4 error in smbd read reply max_send clamp
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.13.0.rc1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-19 12:59 UTC by Sprow
Modified: 2022-06-28 06:51 UTC (History)
1 user (show)

See Also:


Attachments
Patch to exclude NetBIOS header allowance (832 bytes, patch)
2020-07-19 12:59 UTC, Sprow
no flags Details
git-am fix for 4.16.next, 4.15.next. (1.65 KB, patch)
2022-06-08 20:05 UTC, Jeremy Allison
npower: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sprow 2020-07-19 12:59:02 UTC
Created attachment 16134 [details]
Patch to exclude NetBIOS header allowance

There are checks on the request size in both reply_lockread_locked() and reply_read() which attempt to limit the amount read to the smaller of the request and the negotiated SMB1 session max_send value less headers.

However, the calculation uses smb_read (=39) which includes a 4 byte NetBIOS header, while the negotiated max_send value is independent of whether it's wrapped in NetBIOS or not.

This means that any client which does the calculation at their end gets replies which are 4 bytes too short.

Attached patch changes this to use MIN_SMB_SIZE (=35) instead.
Comment 1 Jeremy Allison 2022-06-06 19:29:07 UTC
Hi sprow, this looks good I think. Can you send in a Samba developers declaration, as detailed here:

https://www.samba.org/samba/devel/copyright-policy.html

It will make it much easier for us to accept this fix (we won't have to re-write it).

Thanks !

Jeremy.
Comment 2 Jeremy Allison 2022-06-06 20:22:58 UTC
Developer declaration received.
Comment 3 Samba QA Contact 2022-06-08 19:51:03 UTC
This bug was referenced in samba master:

174a76cc27f25120af5a86bee3f26d9afad87d8f
Comment 4 Jeremy Allison 2022-06-08 20:05:14 UTC
Created attachment 17328 [details]
git-am fix for 4.16.next, 4.15.next.

Cherry-picked from master. Applies cleanly to 4.16.next, 4.15.next.
Comment 5 Noel Power 2022-06-09 10:02:31 UTC
Comment on attachment 17328 [details]
git-am fix for 4.16.next, 4.15.next.

lgtm
Comment 6 Noel Power 2022-06-09 10:03:34 UTC
reassign to Jule for inclusion in 4.16, 4.15
Comment 7 Jule Anger 2022-06-09 10:10:55 UTC
Pushed to autobuild-v4-{16,15}-test.
Comment 8 Samba QA Contact 2022-06-09 11:15:08 UTC
This bug was referenced in samba v4-16-test:

bb60c85153b288b358d288b3ee9f4bceb1304e20
Comment 9 Samba QA Contact 2022-06-10 08:30:05 UTC
This bug was referenced in samba v4-15-test:

d7ea828244830ef70ea406f7c41ce1fc7801c281
Comment 10 Jule Anger 2022-06-10 10:59:15 UTC
Closing out bug report.

Thanks!
Comment 11 Samba QA Contact 2022-06-13 06:56:34 UTC
This bug was referenced in samba v4-16-stable (Release samba-4.16.2):

bb60c85153b288b358d288b3ee9f4bceb1304e20
Comment 12 Samba QA Contact 2022-06-28 06:51:04 UTC
This bug was referenced in samba v4-15-stable (Release samba-4.15.8):

d7ea828244830ef70ea406f7c41ce1fc7801c281