aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Wu <peter@lekensteyn.nl>2016-11-26 14:39:24 +0100
committerPeter Wu <peter@lekensteyn.nl>2016-11-30 13:49:15 +0000
commitff0371e89802047a0b2ed31f127526b945d27069 (patch)
tree3a02542e03d27231e8d5c2ef163dba05794addff
parent1c9e061ea65fb630d18f5e9e4e4ecd3523dea651 (diff)
ssl,dtls: add expert info for overly large record lengths
All TLS and DTLS RFCs (and SSLv3) limit the record length to 2^14, so add expert info if this is exceeded. Spotted in the wild via https://ask.wireshark.org/questions/57641/tls12-record-length-gt-16k-valid Tested with a synthetic pcap having length 2^14+1 using Python: from scapy.all import IP, TCP, UDP, wrpcap len_plus_frag = b'\x40\x01' + 0x4001 * b'\0' wrpcap('bad-record-length.pcap', [ IP()/TCP(sport=2000, dport=443)/(b'\x17\x03\x03' + len_plus_frag), IP()/UDP(sport=2000, dport=853)/(b'\x17\xfe\xfd' + 8*b'\0' + len_plus_frag) ]) Change-Id: I5eac48775333d8d222e013a24a6d06da79892b77 Reviewed-on: https://code.wireshark.org/review/18959 Reviewed-by: Graham Bloice <graham.bloice@trihedral.com> Petri-Dish: Graham Bloice <graham.bloice@trihedral.com> Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org> Reviewed-by: Peter Wu <peter@lekensteyn.nl>
-rw-r--r--epan/dissectors/packet-dtls.c6
-rw-r--r--epan/dissectors/packet-ssl-utils.h10
-rw-r--r--epan/dissectors/packet-ssl.c6
3 files changed, 19 insertions, 3 deletions
diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index 6878efd3f7..21e3ec6226 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -668,6 +668,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
guint8 next_byte;
proto_tree *ti;
proto_tree *dtls_record_tree;
+ proto_item *pi;
SslDataInfo *appl_data;
heur_dtbl_entry_t *hdtbl_entry;
@@ -734,8 +735,11 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
offset += 6;
/* add the length */
- proto_tree_add_uint(dtls_record_tree, hf_dtls_record_length, tvb,
+ pi = proto_tree_add_uint(dtls_record_tree, hf_dtls_record_length, tvb,
offset, 2, record_length);
+ if (record_length > TLS_MAX_RECORD_LENGTH) {
+ expert_add_info(pinfo, pi, &dissect_dtls_hf.ei.record_length_invalid);
+ }
offset += 2; /* move past length field itself */
/*
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index e59dc72d9c..47f3311ecd 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -275,6 +275,9 @@ typedef enum {
#define SSLV2_MAX_SESSION_ID_LENGTH_IN_BYTES 16
+/* Record fragment lengths MUST NOT exceed 2^14 (= 0x4000) */
+#define TLS_MAX_RECORD_LENGTH 0x4000
+
typedef struct _SslCipherSuite {
gint number;
gint kex;
@@ -794,6 +797,7 @@ typedef struct ssl_common_dissect {
expert_field hs_cipher_suites_len_bad;
expert_field hs_sig_hash_algs_bad;
expert_field resumed;
+ expert_field record_length_invalid;
/* do not forget to update SSL_COMMON_LIST_T and SSL_COMMON_EI_LIST! */
} ei;
@@ -898,7 +902,7 @@ ssl_common_dissect_t name = { \
-1, -1, -1, -1, -1, -1, -1, \
}, \
/* ei */ { \
- EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, \
+ EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, \
}, \
}
/* }}} */
@@ -1546,6 +1550,10 @@ ssl_common_dissect_t name = { \
{ & name .ei.resumed, \
{ prefix ".resumed", PI_SEQUENCE, PI_NOTE, \
"This session reuses previously negotiated keys (Session resumption)", EXPFILL } \
+ }, \
+ { & name .ei.record_length_invalid, \
+ { prefix ".record.length.invalid", PI_PROTOCOL, PI_ERROR, \
+ "Record fragment length must not exceed 2^14", EXPFILL } \
}
/* }}} */
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index c50d4bca3b..0e9e6ea137 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -1514,6 +1514,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
guint8 next_byte;
proto_tree *ti;
proto_tree *ssl_record_tree;
+ proto_item *pi;
guint32 available_bytes;
ti = NULL;
@@ -1635,8 +1636,11 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
offset += 2;
/* add the length */
- proto_tree_add_uint(ssl_record_tree, hf_ssl_record_length, tvb,
+ pi = proto_tree_add_uint(ssl_record_tree, hf_ssl_record_length, tvb,
offset, 2, record_length);
+ if (record_length > TLS_MAX_RECORD_LENGTH) {
+ expert_add_info(pinfo, pi, &dissect_ssl3_hf.ei.record_length_invalid);
+ }
offset += 2; /* move past length field itself */
/*