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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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