Hadriel Kaplan
changed
bug 9909
| What |
Removed |
Added |
| Status |
UNCONFIRMED
|
IN_PROGRESS
|
| Ever confirmed |
|
1
|
Comment # 16
on bug 9909
from Hadriel Kaplan
Hopefully I'll get some time to look at this.
Here are some initial notes before I forget.
The bug was introduced by:
Change-Id: I742726027bcab7d25ca4a9ce3a406518db6d272f
Commit: g4b8b83407ac744d114462235a8bcca0d480954c7
The introduced code is de-referencing a previously-saved proto_data, using
p_get_proto_data(). This probably works fine for normal ieee802.11 cases, as
the proto_data had been previously saved to the same "IS_DMG_KEY" in
dissect_ieee80211_common, which all dissectors in packet-ieee80211.c call I
believe.
However, this newly introduced code is in the add_tagged_field() function,
which is called by ieee_80211_add_tagged_parameters()... well, it's called in
other places too, but it's the time when it's called by
ieee_80211_add_tagged_parameters() that causes this crash. And the reason for
that is that ieee_80211_add_tagged_parameters() is called by one dissector
*outside* of packet-ieee80211.c, namely dissect_eapol() calls it in
packet-eapol.c.
So since dissect_eapol never set that IS_DMG_KEY proto_data, the
p_get_proto_data() doesn't find it, returns a NULL, and boom the code crashes.
So the question is if dissect_eapol() should also set the proto_data
before-hand, or not do so and instead make add_tagged_field() treat this acse
as if the isDMG boolean is false. Regardless, we shouldn't be de-referencing
something we don't know is not null, obviously, but the question is what should
happen in this case.
You are receiving this mail because:
- You are watching all bug changes.
