Bug 254737 - Missing size checks in ieee80211_ioctl_setmlme() could lead to an heap overflow
Summary: Missing size checks in ieee80211_ioctl_setmlme() could lead to an heap overflow
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Bjoern A. Zeeb
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-03 08:38 UTC by Tommaso
Modified: 2021-04-12 13:12 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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?