diff options
| author | Jeffrey Smith <whydoubt@gmail.com> | 2015-08-25 18:40:45 -0500 |
|---|---|---|
| committer | Alexis La Goutte <alexis.lagoutte@gmail.com> | 2015-08-26 17:24:01 +0000 |
| commit | bc4487a6fcd1dfe5c3ef602f6eac4651d2b55642 (patch) | |
| tree | f7216630c1ef4fef1ed16ffaefd3690830e31fd5 /epan/dissectors | |
| parent | e1c807c9d19416b2b842601cb24a78a799946bf8 (diff) | |
bootp/dhcp: tighten check for Alcatel extensions
The check for Alcatel extensions in bootp/dhcp packets is very weak,
resulting in some false positives. Then when trying to parse the
suboptions, the result is an error on the packet.
This change eliminates some false positives by adding a test that the
vendor-specific option contents match the encapsulated format described
in section 8.4 of RFC2132.
Change-Id: Ie4188ff900426c2d80a5694fbba5c88385625a61
Reviewed-on: https://code.wireshark.org/review/10267
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Diffstat (limited to 'epan/dissectors')
| -rw-r--r-- | epan/dissectors/packet-bootp.c | 38 |
1 files changed, 36 insertions, 2 deletions
diff --git a/epan/dissectors/packet-bootp.c b/epan/dissectors/packet-bootp.c index 93191af04d..33c76e4089 100644 --- a/epan/dissectors/packet-bootp.c +++ b/epan/dissectors/packet-bootp.c @@ -826,6 +826,7 @@ static int dissect_vendor_pxeclient_suboption(packet_info *pinfo, proto_item *v_ tvbuff_t *tvb, int optoff, int optend); static int dissect_vendor_cablelabs_suboption(packet_info *pinfo, proto_item *v_ti, proto_tree *v_tree, tvbuff_t *tvb, int optoff, int optend); +static gboolean test_encapsulated_vendor_options(tvbuff_t *tvb, int optoff, int optend); static int dissect_vendor_alcatel_suboption(packet_info *pinfo, proto_item *v_ti, proto_tree *v_tree, tvbuff_t *tvb, int optoff, int optend); static int dissect_netware_ip_suboption(packet_info *pinfo, proto_item *v_ti, proto_tree *v_tree, @@ -1730,8 +1731,9 @@ bootp_option(tvbuff_t *tvb, packet_info *pinfo, proto_tree *bp_tree, proto_item ampiplen = tvb_find_guint8(tvb, optoff+nameorglen+1, optlen-nameorglen-1, ',') - (optoff+nameorglen+1); proto_tree_add_item(v_tree, hf_bootp_option43_arubaiap_ampip, tvb, optoff+nameorglen+1, ampiplen, ENC_ASCII|ENC_NA); proto_tree_add_item(v_tree, hf_bootp_option43_arubaiap_password, tvb, optoff+nameorglen+1+ampiplen+1, optlen-(nameorglen+1+ampiplen+1), ENC_ASCII|ENC_NA); - } else if (s_option==58 || s_option==64 || s_option==65 - || s_option==66 || s_option==67) { + } else if ((s_option==58 || s_option==64 || s_option==65 + || s_option==66 || s_option==67) + && test_encapsulated_vendor_options(tvb, optoff, optoff+optlen)) { /* Note that this is a rather weak (permissive) heuristic, */ /* but since it comes last, I guess this is OK. */ /* Add any stronger (less permissive) heuristics before this! */ @@ -3397,6 +3399,38 @@ static const value_string option43_alcatel_app_type_vals[] = { { 0, NULL} }; +/* Look for 'encapsulated vendor-specific options' */ +static gboolean +test_encapsulated_vendor_options(tvbuff_t *tvb, int optoff, int optend) +{ + guint8 subopt; + guint8 subopt_len; + + while (optoff < optend) { + subopt = tvb_get_guint8(tvb, optoff); + optoff++; + + /* Skip padding */ + if (subopt == 0) + continue; + /* We are done, skip any remaining bytes */ + if (subopt == 255) + break; + + /* We expect a length byte next */ + if (optoff >= optend) + return FALSE; + subopt_len = tvb_get_guint8(tvb, optoff); + optoff++; + + /* Check remaining room for suboption in option */ + if (optoff + subopt_len > optoff) + return FALSE; + optoff += subopt_len; + } + return TRUE; +} + static int dissect_vendor_alcatel_suboption(packet_info *pinfo, proto_item *v_ti, proto_tree *v_tree, tvbuff_t *tvb, int optoff, int optend) |