diff mbox series

libertas: Fix two buffer overflows at parsing bss descriptor

Message ID 20191128105104.52920-1-huangwenabc@gmail.com (mailing list archive)
State Accepted
Commit e5e884b42639c74b5b57dc277909915c0aefc8bb
Delegated to: Kalle Valo
Headers show
Series libertas: Fix two buffer overflows at parsing bss descriptor | expand

Commit Message

huangwenabc@gmail.com Nov. 28, 2019, 10:51 a.m. UTC 
From: Wen Huang <huangwenabc@gmail.com>

add_ie_rates() copys rates without checking the length 
in bss descriptor from remote AP.when victim connects to 
remote attacker, this may trigger buffer overflow.
lbs_ibss_join_existing() copys rates without checking the length 
in bss descriptor from remote IBSS node.when victim connects to 
remote attacker, this may trigger buffer overflow.
Fix them by putting the length check before performing copy.

This fix addresses CVE-2019-14896 and CVE-2019-14897.
This also fix build warning of mixed declarations and code.

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Wen Huang <huangwenabc@gmail.com>
---
 drivers/net/wireless/marvell/libertas/cfg.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Kalle Valo Dec. 18, 2019, 6:52 p.m. UTC | #1 
huangwenabc@gmail.com wrote:

> From: Wen Huang <huangwenabc@gmail.com>
> 
> add_ie_rates() copys rates without checking the length 
> in bss descriptor from remote AP.when victim connects to 
> remote attacker, this may trigger buffer overflow.
> lbs_ibss_join_existing() copys rates without checking the length 
> in bss descriptor from remote IBSS node.when victim connects to 
> remote attacker, this may trigger buffer overflow.
> Fix them by putting the length check before performing copy.
> 
> This fix addresses CVE-2019-14896 and CVE-2019-14897.
> This also fix build warning of mixed declarations and code.
> 
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Wen Huang <huangwenabc@gmail.com>

Patch applied to wireless-drivers.git, thanks.

e5e884b42639 libertas: Fix two buffer overflows at parsing bss descriptor
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index 57edfada0..c9401c121 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -273,6 +273,10 @@  add_ie_rates(u8 *tlv, const u8 *ie, int *nrates)
 	int hw, ap, ap_max = ie[1];
 	u8 hw_rate;
 
+	if (ap_max > MAX_RATES) {
+		lbs_deb_assoc("invalid rates\n");
+		return tlv;
+	}
 	/* Advance past IE header */
 	ie += 2;
 
@@ -1717,6 +1721,9 @@  static int lbs_ibss_join_existing(struct lbs_private *priv,
 	struct cmd_ds_802_11_ad_hoc_join cmd;
 	u8 preamble = RADIO_PREAMBLE_SHORT;
 	int ret = 0;
+	int hw, i;
+	u8 rates_max;
+	u8 *rates;
 
 	/* TODO: set preamble based on scan result */
 	ret = lbs_set_radio(priv, preamble, 1);
@@ -1775,9 +1782,12 @@  static int lbs_ibss_join_existing(struct lbs_private *priv,
 	if (!rates_eid) {
 		lbs_add_rates(cmd.bss.rates);
 	} else {
-		int hw, i;
-		u8 rates_max = rates_eid[1];
-		u8 *rates = cmd.bss.rates;
+		rates_max = rates_eid[1];
+		if (rates_max > MAX_RATES) {
+			lbs_deb_join("invalid rates");
+			goto out;
+		}
+		rates = cmd.bss.rates;
 		for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) {
 			u8 hw_rate = lbs_rates[hw].bitrate / 5;
 			for (i = 0; i < rates_max; i++) {