Re: [Wireshark-dev] Possible misuse of match_strval_idx
On Sun, Mar 24, 2013 at 6:11 AM, Pascal Quantin wrote: > Le 24/03/2013 00:57, Evan Huus a écrit : >> On Sat, Mar 23, 2013 at 6:39 PM, Jaap Keuter wrote: >>> On 03/23/2013 10:07 PM, Evan Huus wrote: Am I correct in thinking that in packet-gsm_a_dtap.c around line 6432, if match_strval_idx doesn't find a match then we will access invalid memory because idx will be used as an array index with value -1? Evan >>> Unless gsm_a_dtap_msg_cc_strings contains 64 entries it seems so yes. >> Filed as bug #8517. > > Hi Evan, > > actually those wrong array index will never be used thanks to the > following lines 6503 and 6622: > if (msg_str == NULL) > { >... > } > else > { >use ett_tree > } > and > if (msg_str == NULL) return; > ... > use dtap_msg_fcn > > So the invalid memory accesses will never occur. I believe the access would still have occurred, since the arrays were being indexed to store immediately the value in local variables. The compiler likely optimized that out which made it safe, but a straight literal compiler would have produced a bug. Either way your commit fixed it. > I made it a bit more obvious in r48520. > > Unless you spotted another place in the source code, I suggest closing > bug 8517. I've added the one other location I'm aware of to 8517, as well as a few places where the use of match_strval_idx is unnecessary in the first place (though this is more room for simplification than a bug). Cheers, Evan ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Possible misuse of match_strval_idx
Le 24/03/2013 00:57, Evan Huus a écrit : > On Sat, Mar 23, 2013 at 6:39 PM, Jaap Keuter wrote: >> On 03/23/2013 10:07 PM, Evan Huus wrote: >>> Am I correct in thinking that in packet-gsm_a_dtap.c around line 6432, >>> if match_strval_idx doesn't find a match then we will access invalid >>> memory because idx will be used as an array index with value -1? >>> >>> Evan >> Unless gsm_a_dtap_msg_cc_strings contains 64 entries it seems so yes. > Filed as bug #8517. Hi Evan, actually those wrong array index will never be used thanks to the following lines 6503 and 6622: if (msg_str == NULL) { ... } else { use ett_tree } and if (msg_str == NULL) return; ... use dtap_msg_fcn So the invalid memory accesses will never occur. I made it a bit more obvious in r48520. Unless you spotted another place in the source code, I suggest closing bug 8517. Regards, Pascal. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Possible misuse of match_strval_idx
On Sat, Mar 23, 2013 at 6:39 PM, Jaap Keuter wrote: > On 03/23/2013 10:07 PM, Evan Huus wrote: >> Am I correct in thinking that in packet-gsm_a_dtap.c around line 6432, >> if match_strval_idx doesn't find a match then we will access invalid >> memory because idx will be used as an array index with value -1? >> >> Evan > > Unless gsm_a_dtap_msg_cc_strings contains 64 entries it seems so yes. Filed as bug #8517. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Possible misuse of match_strval_idx
On 03/23/2013 10:07 PM, Evan Huus wrote: > Am I correct in thinking that in packet-gsm_a_dtap.c around line 6432, > if match_strval_idx doesn't find a match then we will access invalid > memory because idx will be used as an array index with value -1? > > Evan Unless gsm_a_dtap_msg_cc_strings contains 64 entries it seems so yes. Thanks, Jaap ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
[Wireshark-dev] Possible misuse of match_strval_idx
Am I correct in thinking that in packet-gsm_a_dtap.c around line 6432, if match_strval_idx doesn't find a match then we will access invalid memory because idx will be used as an array index with value -1? Evan ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe