diff options
author | Guy Harris <guy@alum.mit.edu> | 2015-11-23 21:48:04 -0800 |
---|---|---|
committer | Guy Harris <guy@alum.mit.edu> | 2015-11-24 05:49:24 +0000 |
commit | 2c5997060c1cc91dc3ba964d4494751b50f25470 (patch) | |
tree | c1b50bea99ccd442fa7caa35d4a65bf68c67138a | |
parent | c6efada1b4739dbcb67bc30966980d864a29b0bd (diff) | |
download | wireshark-2c5997060c1cc91dc3ba964d4494751b50f25470.tar.gz |
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
Change-Id: I431d7ed69ac1697bd42c22a37ca1451cfc85c94e
Reviewed-on: https://code.wireshark.org/review/12083
Reviewed-by: Guy Harris <guy@alum.mit.edu>
(cherry picked from commit 921bb07115fbffc081ec56a5022b4a9d58db6d39)
Reviewed-on: https://code.wireshark.org/review/12085
-rw-r--r-- | epan/dissectors/packet-ber.c | 113 |
1 files changed, 82 insertions, 31 deletions
diff --git a/epan/dissectors/packet-ber.c b/epan/dissectors/packet-ber.c index ac8faf153c..fa47f9b934 100644 --- a/epan/dissectors/packet-ber.c +++ b/epan/dissectors/packet-ber.c @@ -3553,49 +3553,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_value( - tree, hf_ber_error, tvb, offset, len, "invalid_generalized_time", - "GeneralizedTime invalid format: %s", - tmpstr); - expert_add_info(actx->pinfo, cause, &ei_ber_invalid_format_generalized_time); - 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) { @@ -3604,6 +3643,18 @@ 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); + expert_add_info(actx->pinfo, cause, &ei_ber_invalid_format_generalized_time); + 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; } |