Bug 222687 - smb_strdupin() does not properly check the length of string duped-in
Summary: smb_strdupin() does not properly check the length of string duped-in
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Conrad Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-29 14:17 UTC by Meng Xu
Modified: 2017-10-30 14:16 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Meng Xu 2017-09-29 14:17:19 UTC
In function smb_strdupin(), it first guesses the length of the input string
(by incrementally read and testing for the NULL terminator) and then copyin
the whole string from userspace. However, given that another user thread can
"scramble" the userspace buffer while smb_strdupin() is in execution,
it might result in a case where the string after second copyin is not 
NULL terminated.

This itself is not yet a very serious issue. However, it does become a bug
later. smb_strdupin() is invoked in smb_usr_t2request() by
t2p->t_name = smb_strdupin(dp->ioc_name, 128);
And later in downstream functions 
smb_t2_request(t2p) --> smb_t2_request_int(t2p)
there is a call to t2p->t_name: nmlen = t2p->t_name ? strlen(t2p->t_name) : 0

Now if t2p->t_name is not NULL terminated, calling strlen(t2p->t_name)
will cause wield behaviors, such as invalid memory accesses.
Comment 1 commit-hook freebsd_committer 2017-09-29 15:54:20 UTC
A commit references this bug:

Author: cem
Date: Fri Sep 29 15:53:26 UTC 2017
New revision: 324102
URL: https://svnweb.freebsd.org/changeset/base/324102

Log:
  netsmb: Fix buggy/racy smb_strdupin()

  smb_strdupin() tried to roll a copyin() based strlen to allocate a buffer
  and then blindly copyin that size.  Of course, a malicious user program
  could simultaneously manipulate the buffer, resulting in a non-terminated
  string being copied.

  Later assumptions in the code rely upon the string being nul-terminated.

  Just use copyinstr() and drop the racy sizing.

  PR:		222687
  Reported by:	Meng Xu <meng.xu AT gatech.edu>
  Security:	possible local DoS
  Sponsored by:	Dell EMC Isilon

Changes:
  head/sys/netsmb/smb_subr.c
Comment 2 Xin LI freebsd_committer 2017-10-08 05:53:41 UTC
Note: MITRE have assigned CVE-2017-15037 for this issue.
Comment 3 Ben Simmons 2017-10-30 14:16:07 UTC
Thnaks for information!
If you need writing help, please visit http://www.123essay.biz/