[PATCH 0/7] smb: client: fix OOB and use-after-free infoleaks in SMB1 FIND responses

[Diego Oliva]

The SMB1 client parses TRANS2_FIND_FIRST2 and TRANS2_FIND_NEXT2
responses without checking them against the number of bytes it
received. A malicious or compromised server can make the client read
past the end of a response, and past the end of the cifs_request
buffer holding it, and send what it read back to the server. Both the
out-of-bounds reads and the use-after-free read are information leaks:
the bytes leave the machine in the client's next FindNext request,
from a position the server picks:

 - LastNameOffset, which the client uses to locate the last entry of
   a response, is only checked against CIFSMaxBufSize, never against
   the response that was actually received, so psrch_inf->last_entry
   can be placed past the end of the allocation.
 - cifs_save_resume_key() parses the entry there without a bound. The
   name length comes from the entry itself, and for SMB_FIND_FILE_UNIX
   from a NUL scan of up to PATH_MAX + 1 UTF-16 units. The name is
   recorded as the resume name of the search, and the next
   CIFSFindNext() rejects only a recorded length of PATH_MAX or more
   before copying the name into its request and sending it, so up to
   4 KiB of memory from beyond the response leaves the machine,
   together with the resume key read from the same entry.
 - When the last entry is rejected, and when a search rewind frees
   the buffer without its FindFirst recording a new name, the resume
   name keeps pointing into a buffer that has been released, and the
   next CIFSFindNext() copies it from there.
 - The response parameters (search handle, entry count, end-of-search
   flag, LastNameOffset) and the data area are read at offsets that
   validate_t2() caps at 1024. It bounds the parameter and data
   counts by the byte count of the response, but nothing ties either
   area to the bytes that were received, so one can begin or end past
   them; those reads stay inside the allocation but can return stale
   bytes.

A FindNext response that reports no entries without ending the search
is enough to make the client send the resume name, and each further
response can do the same. Reaching the code needs a malicious or
compromised server, an explicit vers=1.0 mount of it
(CONFIG_CIFS_ALLOW_INSECURE_LEGACY, default y) and a directory listing
on that mount; SMB1 is not negotiated by default. The same code is in
every maintained stable tree.

Commit f8cf09a53a0d ("smb: client: bound dirent name against end of
SMB response in cifs_filldir"), in v7.2, bounded the name of the
entries cifs_readdir() emits, after the parse; the resume-key path was
left unbounded and the parse itself still ran before any bound.

The fix is one change conceptually, but as a single patch it is well
over the 100 lines with context that stable-kernel-rules.rst allows,
so stable could not take it. This series splits it into patches that
each fix one thing, build and pass checkpatch on their own, and stay
under that limit.

What each patch fixes, and what to backport:

  1/7 fix use-after-free infoleak via the readdir resume name
      The use-after-free. It comes first so that the patches after it
      cannot widen the window it closes. Also resets resume_key with
      the name and skips the zero-length copy in CIFSFindNext().
      Tagged Cc: stable.
  2/7 fix OOB last_entry pointer from unbounded LastNameOffset
      The out-of-bounds pointer, bounded against the response that
      was received rather than against the declared data area. It
      applies on top of 1/7, whose resets sit in its context and
      which makes the paths this patch sends more responses down
      safe. Also records the search handle before the last entry is
      examined, because the new no-entries case would otherwise lose
      the handle. Cc: stable.
  3/7 fix OOB read of the resume name sent in TRANS2_FIND_NEXT2
      Bounds the entry parse in cifs_fill_dirent() for both callers,
      before anything is read. This is what stops a name from being
      recorded and sent from past the end of the response.
      Tagged Cc: stable.
  4/7 fix OOB read in the SMB_FIND_FILE_UNIX name scan
      Caps the NUL scan at the response end; without it the scan
      still runs several KiB past the allocation. Depends on 3/7 for
      the end of the response. A scan that reaches the end of the
      response without finding a terminator is rejected; a name
      longer than the PATH_MAX cap of the scans is still truncated
      there, as before. Cc: stable.
  5/7 reject FIND data areas that run past the received response
      Hardening rather than memory safety: it stops entries being
      parsed from stale bytes inside the allocation, and bounds the
      data area that the nxt_dir_entry() walk starts from and that
      the two query helpers read their entry from. It rejects with
      -EINVAL, as validate_t2() does, logging with cifs_dbg(), and
      adds no tracepoint, so it applies to trees before 6.19.
      Cc: stable.
  6/7 reject short or out-of-range FIND response parameters
      The parameters are read inside the allocation, but the stale
      search handle among them is sent back to the server. Depends on
      5/7, whose declarations it extends. It returns smb_EIO2() with
      two new smb_eio_traces values; backports before v6.19 need
      -EINVAL instead. Cc: stable.
  7/7 drop the redundant name bounds in cifs_filldir()
      Cleanup of the two post-parse checks that 3/7 made redundant.
      Depends on 3/7. Not for stable.

No patch needs one that follows it, so the series can be cut after any
patch. Patches 1/7 to 4/7 are every out-of-bounds and use-after-free
fix; 5/7 stands on its own; 6/7 can follow where smb_EIO2() exists.
Trees before v7.2 need the adjustments noted in the patches that need
them.

cifs_fill_dirent() is shared with the SMB2 readdir path; 3/7 and 4/7
add checks there that a valid SMB2 response cannot fail, since
smb2_parse_query_directory() already walks its entries against the
end of the response.

Left out on purpose: an entry-size check for cifs_query_path_info()
and cifs_backup_query_path_info(), which still read one whole entry
from a data area that 5/7 bounds only as a whole (those reads stay
inside the allocation); passing the end to posix_info_parse() in
cifs_fill_dirent_posix(), which only SMB3.1.1 POSIX mounts reach,
with entries that num_entries() has already validated; and a lower
bound on ParameterOffset and DataOffset, since reads at a low offset
stay inside the received response.

Each patch builds with W=1 without warnings and passes checkpatch.
The out-of-bounds reads and the use-after-free read were reproduced
against a test server with KASAN enabled, by listing a directory on
an SMB1 mount; the splats are in 1/7, 2/7, 3/7 and 4/7, all taken on
the unpatched tree. The one in 4/7 shows the case the preceding
patches leave open, where the entry starts inside the response and
the scan still leaves the allocation.

The series is based on cifs-next at commit f14572c203d5 ("Merge tag
'cifs-fixes-7.3-rc5' of https://git.manguebit.org/linux";). The files it
touches are identical there and in v7.3-rc4.

Diego Oliva (7):
  smb: client: fix use-after-free infoleak via the readdir resume name
  smb: client: fix OOB last_entry pointer from unbounded LastNameOffset
  smb: client: fix OOB read of the resume name sent in TRANS2_FIND_NEXT2
  smb: client: fix OOB read in the SMB_FIND_FILE_UNIX name scan
  smb: client: reject FIND data areas that run past the received
    response
  smb: client: reject short or out-of-range FIND response parameters
  smb: client: drop the redundant name bounds in cifs_filldir()

 fs/smb/client/cifssmb.c | 78 ++++++++++++++++++++++++++++++++++-------
 fs/smb/client/readdir.c | 68 ++++++++++++++++++++++++-----------
 fs/smb/client/trace.h   |  2 ++
 3 files changed, 114 insertions(+), 34 deletions(-)


base-commit: f14572c203d57492e1d4e5d7851a3b143e083b82
-- 
2.39.5