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.
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.
yes, i wouldn’t consider this as severity high, but it’s quite dangerous if chained with a user to root bug.
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.
any update on this?
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?
(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!
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.
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?
In principle, the root user has complete access to the entire system.
(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.
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(-)
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(-)
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(-)
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(-)
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(-)
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(-)