Difference between revisions of "Ksmbd-review"

From SambaWiki
(Created page with "There are various checks that are especially important for SMB3 servers. They fall into multiple categories: - packet processing checks to make sure buffers do not overflow,...")
 
Line 1: Line 1:
 
There are various checks that are especially important for SMB3 servers. They fall into multiple categories:
 
There are various checks that are especially important for SMB3 servers. They fall into multiple categories:
   
- packet processing checks to make sure buffers do not overflow, allowing access to data outside the SMB request
+
*packet processing checks to make sure buffers do not overflow, allowing access to data outside the SMB request
- path processing checks to make sure access to files outside the share is not permitted, these include checks for ".." or "../.." to make sure they do not go outside the share, and checks for symlinks in paths and checks to ensure paths do not allow a processed path "\" or "/" e.g. outside the share
+
*path processing checks to make sure access to files outside the share is not permitted, these include checks for ".." or "../.." to make sure they do not go outside the share, and checks for symlinks in paths and checks to ensure paths do not allow a processed path e.g. with "\" or "/" to go outside the share
- denial of service and resource checks (e.g. ensure that no single client can exhaust the server by opening millions of files, or use so many credits that they submit thousands of simultaneous requests starving other clients)
+
*denial of service and resource checks (e.g. ensure that no single client can exhaust the server by opening millions of files, or use so many credits that they submit thousands of simultaneous requests starving other clients)
   
 
{| class="wikitable"
 
{| class="wikitable"
Line 20: Line 20:
 
|StructureSize field check for 19 SMB3 commands
 
|StructureSize field check for 19 SMB3 commands
 
|
 
|
  +
| already implemented
  +
| smb2_req_struct_sizes and smb2_get_data_area_len in smb2misc.c
 
|-
  +
|check for overflow of SET_INFO command
 
|
 
|
  +
| patches submitted, partially reviewed
  +
| "ksmbd: add request buffer validation in smb2_set_info"
 
|-
  +
|check for overflow of FSCTL command
 
|
 
|
  +
| patch submitted, partially reviewed
  +
| "ksmbd: add validation in smb2_ioctl", smb2pdu.c
 
|-
 
|-
 
|Generic check for packet overflow
 
|Generic check for packet overflow
Line 30: Line 40:
 
|Check for open context overflow
 
|Check for open context overflow
 
|
 
|
  +
| patch submitted, partially reviewed
  +
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", oplock.c smb2pdu.c
 
|-
  +
|Check for overflow of ACL
 
|
 
|
  +
| patch submitted, partially reviewed
|
 
  +
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", smbacl.c (parse_dacl and parse_sec_desc functions)
|-
 
 
|Check for negotiate context overflow
 
|Check for negotiate context overflow
 
|
 
|
Line 50: Line 64:
 
|checks for .. escaping the share
 
|checks for .. escaping the share
 
|
 
|
  +
| patch submitted, reviewed
|
 
 
|
 
|
 
|-
 
|-
 
|checks for symlinks in path components
 
|checks for symlinks in path components
 
|
 
|
  +
| patch submitted, partially reviewed
|
 
 
|
 
|
 
|-
 
|-

Revision as of 18:12, 20 September 2021

There are various checks that are especially important for SMB3 servers. They fall into multiple categories:

  • packet processing checks to make sure buffers do not overflow, allowing access to data outside the SMB request
  • path processing checks to make sure access to files outside the share is not permitted, these include checks for ".." or "../.." to make sure they do not go outside the share, and checks for symlinks in paths and checks to ensure paths do not allow a processed path e.g. with "\" or "/" to go outside the share
  • denial of service and resource checks (e.g. ensure that no single client can exhaust the server by opening millions of files, or use so many credits that they submit thousands of simultaneous requests starving other clients)
Packet processing checks in ksmbd
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
SMB3 header validation
StructureSize field check for 19 SMB3 commands already implemented smb2_req_struct_sizes and smb2_get_data_area_len in smb2misc.c
check for overflow of SET_INFO command patches submitted, partially reviewed "ksmbd: add request buffer validation in smb2_set_info"
check for overflow of FSCTL command patch submitted, partially reviewed "ksmbd: add validation in smb2_ioctl", smb2pdu.c
Generic check for packet overflow
Check for open context overflow patch submitted, partially reviewed See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", oplock.c smb2pdu.c
Check for overflow of ACL patch submitted, partially reviewed See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", smbacl.c (parse_dacl and parse_sec_desc functions) Check for negotiate context overflow
Path processing checks in ksmbd
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
checks for .. escaping the share patch submitted, reviewed
checks for symlinks in path components patch submitted, partially reviewed
checks for resolved path never being / or \ (outside the share)


Denial of Service checks
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
checks for opening too many files
checks for allowing too many requests (crediting)