osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

TLS: fix DISSECTOR_ASSERT for zero-length records fragments 

When decrypt_ssl3_record is called with a record length of zero, it will
pass NULL to ssl_data_set because tvb_get_ptr(..., 0) yields NULL. That
triggers a DISSECTOR_ASSERT. Fix this and add expert info while at it.

Bug: 15780
Change-Id: I727b511aa48b6e1aeb20a441d1eb9d3627a74413
Reviewed-on: https://code.wireshark.org/review/33203
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
This commit is contained in:
Peter Wu 2019-05-15 01:30:29 +01:00 committed by Alexis La Goutte
parent 11110ae11e
commit ab1a7cc4a5
4 changed files with 26 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
epan/dissectors/packet-dtls.c
View File

@ -775,7 +775,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
if (decrypted) {
add_new_data_source(pinfo, decrypted, "Decrypted DTLS");
}
ssl_check_record_length(&dissect_dtls_hf, pinfo, record_length, length_pi, session->version, decrypted);
ssl_check_record_length(&dissect_dtls_hf, pinfo, (ContentType)content_type, record_length, length_pi, session->version, decrypted);
switch ((ContentType) content_type) {

17
epan/dissectors/packet-tls-utils.c
View File

@ -7546,6 +7546,7 @@ ssl_try_set_version(SslSession *session, SslDecryptSession *ssl,
void
ssl_check_record_length(ssl_common_dissect_t *hf, packet_info *pinfo,
ContentType content_type,
guint record_length, proto_item *length_pi,
guint16 version, tvbuff_t *decrypted_tvb)
{
@ -7557,6 +7558,22 @@ ssl_check_record_length(ssl_common_dissect_t *hf, packet_info *pinfo,
/* RFC 5246, Section 6.2.3: TLSCiphertext.fragment length MUST NOT exceed 2^14 + 2048 */
max_expansion = 2048;
}
/*
* RFC 5246 (TLS 1.2), Section 6.2.1 forbids zero-length Handshake, Alert
* and ChangeCipherSpec.
* RFC 6520 (Heartbeats) does not mention zero-length Heartbeat fragments,
* so assume it is permitted.
* RFC 6347 (DTLS 1.2) does not mention zero-length fragments either, so
* assume TLS 1.2 requirements.
*/
if (record_length == 0 &&
(content_type == SSL_ID_CHG_CIPHER_SPEC ||
content_type == SSL_ID_ALERT ||
content_type == SSL_ID_HANDSHAKE)) {
expert_add_info_format(pinfo, length_pi, &hf->ei.record_length_invalid,
"Zero-length %s fragments are not allowed",
val_to_str_const(content_type, ssl_31_content_type, "unknown"));
}
if (record_length > TLS_MAX_RECORD_LENGTH + max_expansion) {
expert_add_info_format(pinfo, length_pi, &hf->ei.record_length_invalid,
"TLSCiphertext length MUST NOT exceed 2^14 + %u", max_expansion);

3
epan/dissectors/packet-tls-utils.h
View File

@ -1022,6 +1022,7 @@ ssl_end_vector(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo, prot
extern void
ssl_check_record_length(ssl_common_dissect_t *hf, packet_info *pinfo,
ContentType content_type,
guint record_length, proto_item *length_pi,
guint16 version, tvbuff_t *decrypted_tvb);
@ -2128,7 +2129,7 @@ ssl_common_dissect_t name = { \
}, \
{ & name .ei.record_length_invalid, \
{ prefix ".record.length.invalid", PI_PROTOCOL, PI_ERROR, \
"Record fragment length is too large", EXPFILL } \
"Record fragment length is too small or too large", EXPFILL } \
}
/* }}} */

9
epan/dissectors/packet-tls.c
View File

@ -1016,7 +1016,10 @@ tls_save_decrypted_record(packet_info *pinfo, gint record_id, SslDecryptSession
}
content_type = data[--datalen];
if (datalen == 0) {
/* XXX should we remember that the decrypted contents was zero-length? */
/*
* XXX zero-length Handshake fragments are forbidden by RFC 8446,
* Section 5.1. Empty Application Data fragments are allowed though.
*/
return;
}
}
@ -1916,7 +1919,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
* handshake records in the same frame).
* In TLS 1.3, only "Application Data" records are encrypted.
*/
if (ssl && (session->version != TLSV1DOT3_VERSION || content_type == SSL_ID_APP_DATA)) {
if (ssl && record_length && (session->version != TLSV1DOT3_VERSION || content_type == SSL_ID_APP_DATA)) {
gboolean decrypt_ok = FALSE;
/* Try to decrypt TLS 1.3 early data first */
@ -1954,7 +1957,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
proto_item_set_generated(ti);
}
}
ssl_check_record_length(&dissect_ssl3_hf, pinfo, record_length, length_pi, version, decrypted);
ssl_check_record_length(&dissect_ssl3_hf, pinfo, (ContentType)content_type, record_length, length_pi, version, decrypted);
switch ((ContentType) content_type) {
case SSL_ID_CHG_CIPHER_SPEC: