cfg80211_find_vendor_ie() was checking only that the vendor IE would
fit in the remaining IEs buffer. If a corrupt includes a vendor IE
that is too small, we could potentially overrun the IEs buffer.
Fix this by checking that the vendor IE fits in the reported IE length
field and skip it otherwise.
Reported-by: Jouni Malinen <[email protected]>
Cc: Johannes Berg <[email protected]>
Signed-off-by: Luciano Coelho <[email protected]>
---
net/wireless/scan.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 45f1618..4582801 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -277,14 +277,18 @@ const u8 *cfg80211_find_vendor_ie(unsigned int oui, u8 oui_type,
if (!pos)
return NULL;
- if (end - pos < sizeof(*ie))
- return NULL;
-
ie = (struct ieee80211_vendor_ie *)pos;
+
+ /* make sure we can access ie->len */
+ BUILD_BUG_ON(offsetof(struct ieee80211_vendor_ie, len) >= 2);
+
+ if (ie->len < sizeof(*ie))
+ goto cont;
+
ie_oui = ie->oui[0] << 16 | ie->oui[1] << 8 | ie->oui[2];
if (ie_oui == oui && ie->oui_type == oui_type)
return pos;
-
+cont:
pos += 2 + ie->len;
}
return NULL;
--
1.7.10.4
On Tue, 2013-02-12 at 20:11 +0200, Luciano Coelho wrote:
> cfg80211_find_vendor_ie() was checking only that the vendor IE would
> fit in the remaining IEs buffer. If a corrupt includes a vendor IE
> that is too small, we could potentially overrun the IEs buffer.
>
> Fix this by checking that the vendor IE fits in the reported IE length
> field and skip it otherwise.
Applied. I changed the BUILD_BUG_ON to be != 1 since it has to be that,
but if one breaks that ...
johannes
On Wed, 2013-02-13 at 10:15 +0100, Johannes Berg wrote:
> On Tue, 2013-02-12 at 20:11 +0200, Luciano Coelho wrote:
> > cfg80211_find_vendor_ie() was checking only that the vendor IE would
> > fit in the remaining IEs buffer. If a corrupt includes a vendor IE
> > that is too small, we could potentially overrun the IEs buffer.
> >
> > Fix this by checking that the vendor IE fits in the reported IE length
> > field and skip it otherwise.
>
> Applied. I changed the BUILD_BUG_ON to be != 1 since it has to be that,
> but if one breaks that ...
Okay. But it actually needs to be 1 in the cfg80211_find_ie() function,
not here. Here we just need to make sure that ie->len is guaranteed to
fit in what we got. cfg80211_find_ie() guarantees that we have at least
2 bytes, thus the >= 2 I used.
Anyway, this is all *extremely* nitpicky already. :P
--
Luca.