diff options
| author | Peter Wu <peter@lekensteyn.nl> | 2017-02-01 11:49:30 +0100 |
|---|---|---|
| committer | Alexis La Goutte <alexis.lagoutte@gmail.com> | 2017-02-01 13:40:30 +0000 |
| commit | bf14d8faf8d6de5e841fe8f5ab5210b1b7a426a4 (patch) | |
| tree | d6ace698516e03e1a65590b0fe774bdd8395d1dc /epan/dissectors/packet-dtls.c | |
| parent | 5dde07c8fdb2e17c4d4142427fe29f7db66c1965 (diff) | |
| download | wireshark-bf14d8faf8d6de5e841fe8f5ab5210b1b7a426a4.tar.gz | |
dtls: avoid possible NULL deref
"decoder->flow" could result in a NULL pointer dereference if a null
cipher was in use (caught by Clang static analyzer).
Answering the questions:
- DTLS records fragments do not need to be reassembled, thus there is no
flow. The Handshake messages have their own fragment_offset field and
thus there is no need to maintain an extra flow.
- Actually one datagram can contain multiple records (RFC 6347, 4.1.1),
but this is not implemented yet. The key can however not be "0"
though, it must match the offsets from ssl_get_record_info.
Fixes: v2.3.0rc0-2152-g77404250d5 ("(D)TLS: consolidate and simplify decrypted records handling")
Change-Id: Iac367a68a2936559cd5d557f877c5598114cadca
Reviewed-on: https://code.wireshark.org/review/19892
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Diffstat (limited to 'epan/dissectors/packet-dtls.c')
| -rw-r--r-- | epan/dissectors/packet-dtls.c | 12 |
1 files changed, 3 insertions, 9 deletions
diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c index 0f526fa179..f3a9ccb051 100644 --- a/epan/dissectors/packet-dtls.c +++ b/epan/dissectors/packet-dtls.c @@ -567,8 +567,7 @@ dtls_is_null_cipher(guint cipher ) static gboolean decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryptSession *ssl, - guint8 content_type, guint16 record_version, guint16 record_length, - gboolean allow_fragments) + guint8 content_type, guint16 record_version, guint16 record_length) { gboolean success; SslDecoder *decoder; @@ -633,13 +632,9 @@ decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryp const guchar *data = dtls_decrypted_data.data; guint datalen = dtls_decrypted_data_avail; - // XXX does DTLS have a flow like TLS? - - // XXX in DTLS, there is only one record per datagram right? Then the "key" - // (tvb_raw_offset(tvb)+offset) should not really be needed and can be "0"? ssl_add_record_info(proto_dtls, pinfo, data, datalen, tvb_raw_offset(tvb)+offset, - allow_fragments ? decoder->flow : NULL, (ContentType)content_type); + NULL, (ContentType)content_type); } return success; } @@ -793,8 +788,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, /* try to decrypt record on the first pass, if possible. Store decrypted * record for later usage (without having to decrypt again). */ if (ssl) { - decrypt_dtls_record(tvb, pinfo, offset, ssl, content_type, version, record_length, - content_type == SSL_ID_APP_DATA || content_type == SSL_ID_HANDSHAKE); + decrypt_dtls_record(tvb, pinfo, offset, ssl, content_type, version, record_length); } decrypted = ssl_get_record_info(tvb, proto_dtls, pinfo, tvb_raw_offset(tvb)+offset, &record); if (decrypted) { |