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: 2019-02-09 08:11 UTC (History)
13 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 freebsd_triage 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 freebsd_triage 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
MARKED AS SPAM
Comment 4 vali gholami 2017-12-17 07:13:16 UTC
MARKED AS SPAM
Comment 5 omejames 2018-01-25 10:23:17 UTC
MARKED AS SPAM
Comment 6 Michael Osipov 2018-02-06 13:09:35 UTC
Why hasn't this beeen MFCed?
Comment 7 commit-hook freebsd_committer freebsd_triage 2018-04-01 16:44:07 UTC
A commit references this bug:

Author: markj
Date: Sun Apr  1 16:43:30 UTC 2018
New revision: 331867
URL: https://svnweb.freebsd.org/changeset/base/331867

Log:
  MFC r324102 (by cem):
  netsmb: Fix buggy/racy smb_strdupin()

  PR:	222687

Changes:
_U  stable/11/
  stable/11/sys/netsmb/smb_subr.c
Comment 8 Kuha 2018-04-25 22:40:30 UTC
MARKED AS SPAM
Comment 9 maria denial 2018-05-18 05:59:47 UTC
MARKED AS SPAM
Comment 10 deletehistoryfree 2018-05-22 19:51:34 UTC
MARKED AS SPAM
Comment 11 heather rosado 2018-08-02 05:57:58 UTC
MARKED AS SPAM
Comment 12 pedro 2018-08-19 21:26:35 UTC
MARKED AS SPAM
Comment 13 John Martin 2019-02-09 08:11:30 UTC
MARKED AS SPAM