Bug 254737

Summary:	Missing size checks in ieee80211_ioctl_setmlme() could lead to an heap overflow
Product:	Base System	Reporter:	Tommaso <cutesmilee.research>
Component:	kern	Assignee:	Bjoern A. Zeeb <bz>
Status:	Closed FIXED
Severity:	Affects Many People	CC:	bz, chris, emaste, markj, philip, secteam
Priority:	---	Keywords:	needs-qa
Version:	CURRENT	Flags:	koobs: maintainer-feedback? (secteam) koobs: mfc-stable13? koobs: mfc-stable12? koobs: mfc-stable11?
Hardware:	Any
OS:	Any

Description Tommaso 2021-04-03 08:38:53 UTC

ieee80211_ioctl_set80211() can be called with ioctl() with SIOCS80211 type by root user (there's ieee80211_priv_check_vap_manage() which checks for PRIV_NET80211_VAP_MANAGE, and from my understading it's root only).
with i_type set to IEEE80211_IOC_MLME, ieee80211_ioctl_setmlme() will be called using user provided ireq and vap structs.
ireq->i_data (which is user controlled) is copied to a stack struct struct ieee80211req_mlme, and if (vap->iv_opmode is set to IEEE80211_M_IBSS or IEEE80211_M_AHDEMO and mlme.im_op is set to IEEE80211_MLME_ASSOC, setmlme_assoc_adhoc() will be called with mlme fields which are user controlled (mlme.im_macaddr, mlme.im_ssid_len and mlme.im_ssid).
now struct ieee80211_scan_req is allocated in the heap (it's 128 bytes large), and some memcpy()s are performed, the last one is the most interesting one since it copies user provided ssid, with ssid_len (which is a uint8_t so can be 255 at max) to the previously allocated struct (actually to the last field, which is a 108 bytes struct), since the size can be 255, and there're no bounds checks (just one that checks that it isn't 0) an heap overflow up to 147 bytes (255 - 108) can be triggered, and I think it's big enough to be exploited.
at the moment I wasn’t able to make a poc.

Comment 1 Bjoern A. Zeeb freebsd_committer

2021-04-03 10:08:39 UTC

Thanks.   Not having looked but from your description, yes it's root only so only a potential problem when running untrusted binaries as root.

I'll go and have a look and fix.

Comment 2 Tommaso 2021-04-03 10:10:31 UTC

yes, i wouldn’t consider this as severity high, but it’s quite dangerous if chained with a user to root bug.

Comment 3 Tommaso 2021-04-03 10:27:51 UTC

also there’re two memcpy()s with the same size before the oob memcpy(), they also can go oob, but on the stack.
adding a size check like KASSERT(ssid_len =< 108) should be enough for the heap overflow, I don’t know about the stack ones, because I haven’t investigated much on them.

Comment 4 Tommaso 2021-04-06 06:03:28 UTC

any update on this?

Comment 5 Bjoern A. Zeeb freebsd_committer

2021-10-06 19:13:57 UTC

From my reading of the code, the size check needs to be ssid_len > 32 (IEEE80211_NWID_LEN) then EINVAL as SSID is at most 32 long both in the ioctl and the function argument.

https://reviews.freebsd.org/D32341

With that it seems none of the follow-up problems can be hit.  Would you agree?

Comment 6 Tommaso 2021-10-06 20:10:08 UTC

(In reply to Bjoern A. Zeeb from comment #5)
yes, that check seems to fix the issue.
thanks for fixing it!
I’ve a question, would it be possible to request a CVE-id for this vulnerability? thanks in advance!

Comment 7 Philip Paeps freebsd_committer

2021-10-07 04:37:07 UTC

Please check our criteria for issuing security advisories:

https://www.freebsd.org/security/

From my reading of this issue, it requires root privilege to exploit.  Unless there are considerable aggravating circumstances, we would not issue a security advisory for this.

If this bug significantly affects users of released versions of FreeBSD, we could issue an errata notice.

Are there aggravating circumstances?  I wouldn't consider the possibility of chaining with a user to root bug a valid argument by itself.  The root to user bug would clearly merit a security advisory and CVE but not this heap overflow by itself.

Comment 8 Tommaso 2021-10-07 05:01:03 UTC

I did a bit of research, and seems like that (unlike iOS for example) the root user can write to other processes’ memory, can you confirm this?

Comment 9 Philip Paeps freebsd_committer

2021-10-07 10:22:15 UTC

In principle, the root user has complete access to the entire system.

Comment 10 Tommaso 2021-10-07 11:05:10 UTC

(In reply to Philip Paeps from comment #9)
well if having kernel code exec / kernel arbitrary r/w can’t cause other problems that the root user already can do, this makes sense.

Comment 11 commit-hook freebsd_committer

2021-10-08 10:25:27 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=526370fb85db4b659cff4625eb2f379acaa4a1a8

commit 526370fb85db4b659cff4625eb2f379acaa4a1a8
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
CommitDate: 2021-10-08 10:23:31 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)
    MFC after:      3 days
    Reviewed by:    emaste, adrian
    Differential Revision: https://reviews.freebsd.org/D32341

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 12 commit-hook freebsd_committer

2021-11-19 00:04:01 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ab5678c6c0d0b28feafdb2fd397866d6088f37d8

commit ab5678c6c0d0b28feafdb2fd397866d6088f37d8
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
CommitDate: 2021-11-19 00:01:25 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)

    (cherry picked from commit 526370fb85db4b659cff4625eb2f379acaa4a1a8)
    (cherry picked from commit 0525ece3554edce14fa68a7fb61078ae2110c44b)

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 13 commit-hook freebsd_committer

2022-02-15 15:45:54 UTC

A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f4d0e8787a09f4cdfb856924aaca97f1c78b65b1

commit f4d0e8787a09f4cdfb856924aaca97f1c78b65b1
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2022-02-15 15:03:09 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)

    (cherry picked from commit 526370fb85db4b659cff4625eb2f379acaa4a1a8)
    (cherry picked from commit 0525ece3554edce14fa68a7fb61078ae2110c44b)
    (cherry picked from commit ab5678c6c0d0b28feafdb2fd397866d6088f37d8)

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 14 commit-hook freebsd_committer

2022-03-15 18:14:33 UTC

A commit in branch releng/13.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6eb932a969139b9f42f0ad130bda36f0d709e2a9

commit 6eb932a969139b9f42f0ad130bda36f0d709e2a9
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-03-15 17:45:27 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)

    (cherry picked from commit 526370fb85db4b659cff4625eb2f379acaa4a1a8)
    (cherry picked from commit 0525ece3554edce14fa68a7fb61078ae2110c44b)
    (cherry picked from commit ab5678c6c0d0b28feafdb2fd397866d6088f37d8)

    Approved by:    so
    Security:       FreeBSD-SA-22:02.wifi

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 15 commit-hook freebsd_committer

2022-03-15 18:16:37 UTC

A commit in branch releng/12.3 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=46f1fb82372e38a2e32bf47522cfe1f67de144b2

commit 46f1fb82372e38a2e32bf47522cfe1f67de144b2
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-03-15 18:16:01 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)

    (cherry picked from commit 526370fb85db4b659cff4625eb2f379acaa4a1a8)
    (cherry picked from commit 0525ece3554edce14fa68a7fb61078ae2110c44b)
    (cherry picked from commit ab5678c6c0d0b28feafdb2fd397866d6088f37d8)
    (cherry picked from commit f4d0e8787a09f4cdfb856924aaca97f1c78b65b1)

    Approved by:    so
    Security:       FreeBSD-SA-22:02.wifi

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 16 commit-hook freebsd_committer

2022-03-15 18:17:43 UTC

A commit in branch releng/12.2 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b2107e60f62ed2a232900d77ec54804228d1bfc8

commit b2107e60f62ed2a232900d77ec54804228d1bfc8
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-10-06 18:41:37 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-03-15 17:39:55 +0000

    net80211: proper ssid length check in setmlme_assoc_adhoc()

    A user supplied SSID length is used without proper checks in
    setmlme_assoc_adhoc() which can lead to copies beyond the end
    of the user supplied buffer.
    The ssid is a fixed size array for the ioctl and the argument
    to setmlme_assoc_adhoc().
    In addition to an ssid_len check of 0 also error in case the
    ssid_len is larger than the size of the ssid array to prevent
    problems.

    PR:             254737
    Reported by:    Tommaso (cutesmilee.research protonmail.com)

    (cherry picked from commit 526370fb85db4b659cff4625eb2f379acaa4a1a8)
    (cherry picked from commit 0525ece3554edce14fa68a7fb61078ae2110c44b)
    (cherry picked from commit ab5678c6c0d0b28feafdb2fd397866d6088f37d8)
    (cherry picked from commit f4d0e8787a09f4cdfb856924aaca97f1c78b65b1)

    Approved by:    so
    Security:       FreeBSD-SA-22:02.wifi

 sys/net80211/ieee80211_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)