From 8f4a7b1ffd56b4f4010fa2c41f8a5e827b38beb7 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Mon, 23 Nov 2015 21:48:04 -0800 Subject: [PATCH 5/6] Check *how many* fields sscanf() found. In the code that parses a GeneralizedTime field, don't assume that all fields were found; check the return value from sscanf(). This should clean up a fuzz failure on the 2.0 buildbot: https://buildbot.wireshark.org/wireshark-2.0/builders/Fuzz%20Test/builds/13/steps/valgrind-wireshark/logs/stdio Conflicts: epan/dissectors/packet-ber.c Change-Id: I431d7ed69ac1697bd42c22a37ca1451cfc85c94e Reviewed-on: https://code.wireshark.org/review/12083 Reviewed-by: Guy Harris (cherry picked from commit 921bb07115fbffc081ec56a5022b4a9d58db6d39) Reviewed-on: https://code.wireshark.org/review/12085 (cherry picked from commit 2c5997060c1cc91dc3ba964d4494751b50f25470) Reviewed-on: https://code.wireshark.org/review/13758 Reviewed-by: Balint Reczey Reviewed-on: https://code.wireshark.org/review/14248 --- epan/dissectors/packet-ber.c | 109 ++++++++++++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 28 deletions(-) diff --git a/epan/dissectors/packet-ber.c b/epan/dissectors/packet-ber.c index 727647b..528f384 100644 --- a/epan/dissectors/packet-ber.c +++ b/epan/dissectors/packet-ber.c @@ -4175,46 +4175,88 @@ dissect_ber_GeneralizedTime(gboolean implicit_tag, asn1_ctx_t *actx, proto_tree ret = sscanf( tmpstr, "%14d%1[.,+-Z]%4d%1[+-Z]%4d", &tmp_int, first_delim, &first_digits, second_delim, &second_digits); /* tmp_int does not contain valid value bacause of overflow but we use it just for format checking */ if (ret < 1) { - cause = proto_tree_add_string_format(tree, hf_ber_error, tvb, offset, len, "invalid_generalized_time", "BER Error: GeneralizedTime invalid format: %s", tmpstr); - expert_add_info_format(actx->pinfo, cause, PI_MALFORMED, PI_WARN, "BER Error: GeneralizedTime invalid format"); - if (decode_unexpected) { - proto_tree *unknown_tree = proto_item_add_subtree(cause, ett_ber_unknown); - dissect_unknown_ber(actx->pinfo, tvb, offset, unknown_tree); - } - return end_offset; + /* Nothing matched */ + goto invalid; } - switch (first_delim[0]) { - case '.': - case ',': - strptr += g_snprintf(strptr, 5, "%c%.3d", first_delim[0], first_digits); - switch (second_delim[0]) { + if (ret >= 2) { + /* + * We saw the date+time and the first delimiter. + * + * Either: + * + * it's '.' or ',', in which case we have a fraction of a + * minute or hour; + * + * it's '+' or '-', in which case we have an offset from UTC; + * + * it's 'Z', in which case the time is UTC. + */ + switch (first_delim[0]) { + case '.': + case ',': + /* + * Fraction of a minute or an hour. + */ + if (ret == 2) { + /* + * We saw the decimal sign, but didn't see the fraction. + */ + goto invalid; + } + strptr += g_snprintf(strptr, 5, "%c%.3d", first_delim[0], first_digits); + if (ret >= 4) { + /* + * We saw the fraction and the second delimiter. + * + * Either: + * + * it's '+' or '-', in which case we have an offset + * from UTC; + * + * it's 'Z', in which case the time is UTC. + */ + switch (second_delim[0]) { + case '+': + case '-': + if (ret == 4) { + /* + * We saw the + or -, but didn't see the offset + * from UTC. + */ + goto invalid; + } + g_snprintf(strptr, 12, " (UTC%c%.4d)", second_delim[0], second_digits); + break; + case 'Z': + g_snprintf(strptr, 7, " (UTC)"); + break; + default: + /* handle the malformed field */ + break; + } + } + break; case '+': case '-': - g_snprintf(strptr, 12, " (UTC%c%.4d)", second_delim[0], second_digits); + /* + * Offset from UTC. + */ + if (ret == 2) { + /* + * We saw the + or -1, but didn't see the offset. + */ + goto invalid; + } + g_snprintf(strptr, 12, " (UTC%c%.4d)", first_delim[0], first_digits); break; case 'Z': g_snprintf(strptr, 7, " (UTC)"); break; - case 0: - break; default: /* handle the malformed field */ break; } - break; - case '+': - case '-': - g_snprintf(strptr, 12, " (UTC%c%.4d)", first_delim[0], first_digits); - break; - case 'Z': - g_snprintf(strptr, 7, " (UTC)"); - break; - case 0: - break; - default: - /* handle the malformed field */ - break; } if(hf_id >= 0){ @@ -4223,6 +4265,17 @@ dissect_ber_GeneralizedTime(gboolean implicit_tag, asn1_ctx_t *actx, proto_tree offset+=len; return offset; + +invalid: + cause = proto_tree_add_string_format_value( + tree, hf_ber_error, tvb, offset, len, "invalid_generalized_time", + "GeneralizedTime invalid format: %s", + tmpstr); + if (decode_unexpected) { + proto_tree *unknown_tree = proto_item_add_subtree(cause, ett_ber_unknown); + dissect_unknown_ber(actx->pinfo, tvb, offset, unknown_tree); + } + return end_offset; } -- 2.1.4