Commit Graph

16 Commits

Author SHA1 Message Date
Adam Langley
abae631fb9 Remove some duplicate DTLS code.
In a couple of functions, a sequence number would be calculated twice.

Additionally, in |dtls1_process_out_of_seq_message|, we know that
|frag_len| <= |msg_hdr->msg_len| so the later tests for |frag_len <
msg_hdr->msg_len| can be more clearly written as |frag_len !=
msg_hdr->msg_len|, since that's the only remaining case.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's d345a24569edf0a966b3d6eaae525f0ca4c5e570)

Change-Id: I038f9f01a1d9379f1ee058b231d80e8b9ce6c2d7
Reviewed-on: https://boringssl-review.googlesource.com/1438
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:32 +00:00
Matt Caswell
e24f686e31 Same fix as in dtls1_process_out_of_seq_message.
Applying same fix as in dtls1_process_out_of_seq_message. A truncated
DTLS fragment would cause *ok to be clear, but the return value would
still be the number of bytes read.

Problem identified by Emilia Käsper, based on previous issue/patch by Adam
Langley.

Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 3d5dceac430d7b9b273331931d4d2303f5a2256f)

Change-Id: Ibe30716266e2ee1489c98b922cf53edda096c23c
Reviewed-on: https://boringssl-review.googlesource.com/1437
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:21 +00:00
Adam Langley
8506609ca3 Fix return code for truncated DTLS fragment.
Previously, a truncated DTLS fragment in
|dtls1_process_out_of_seq_message| would cause *ok to be cleared, but
the return value would still be the number of bytes read. This would
cause |dtls1_get_message| not to consider it an error and it would
continue processing as normal until the calling function noticed that
*ok was zero.

I can't see an exploit here because |dtls1_get_message| uses
|s->init_num| as the length, which will always be zero from what I can
see.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's aad61c0a57a3b6371496034db61675abcdb81811.)

Change-Id: I2fb0ea93b6e812e19723ada3351f842cc7b2fa91
Reviewed-on: https://boringssl-review.googlesource.com/1436
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:12 +00:00
Adam Langley
e951ff4fc3 Fix memory leak from zero-length DTLS fragments.
The |pqueue_insert| function can fail if one attempts to insert a
duplicate sequence number. When handling a fragment of an out of
sequence message, |dtls1_process_out_of_seq_message| would not call
|dtls1_reassemble_fragment| if the fragment's length was zero. It would
then allocate a fresh fragment and attempt to insert it, but ignore the
return value, leaking the fragment.

This allows an attacker to exhaust the memory of a DTLS peer.

Fixes CVE-2014-3507

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 8ca4c4b25e050b881f3aad7017052842b888722d.)

Change-Id: I387e3f6467a0041f6367965ed3c1ad4377b9ac08
Reviewed-on: https://boringssl-review.googlesource.com/1435
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:00 +00:00
Matt Caswell
2306fe5ff5 Fix DTLS handshake message size checks.
In |dtls1_reassemble_fragment|, the value of
|msg_hdr->frag_off+frag_len| was being checked against the maximum
handshake message size, but then |msg_len| bytes were allocated for the
fragment buffer. This means that so long as the fragment was within the
allowed size, the pending handshake message could consume 16MB + 2MB
(for the reassembly bitmap). Approx 10 outstanding handshake messages
are allowed, meaning that an attacker could consume ~180MB per DTLS
connection.

In the non-fragmented path (in |dtls1_process_out_of_seq_message|), no
check was applied.

Fixes CVE-2014-3506

Wholly based on patch by Adam Langley with one minor amendment.

Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 0598468fc04fb0cf2438c4ee635b587aac1bcce6)

Change-Id: I4849498eabb45ec973fcb988d639b23145891e25
Reviewed-on: https://boringssl-review.googlesource.com/1434
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:08:49 +00:00
Matt Caswell
3873f6f33d Added comment for the frag->reassembly == NULL case as per feedback from Emilia
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's ea7cb5397457c59554155935b677a1dab23bd864)

Change-Id: Idd5ed233028c42d2b921deb424381aad88a0aa84
Reviewed-on: https://boringssl-review.googlesource.com/1433
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 18:20:41 +00:00
Adam Langley
d06afe40ab Avoid double free when processing DTLS packets.
The |item| variable, in both of these cases, may contain a pointer to a
|pitem| structure within |s->d1->buffered_messages|. It was being freed
in the error case while still being in |buffered_messages|. When the
error later caused the |SSL*| to be destroyed, the item would be double
freed.

Thanks to Wah-Teh Chang for spotting that the fix in 1632ef74 was
inconsistent with the other error paths (but correct).

Fixes CVE-2014-3505

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 49850075555893c9c60d5b981deb697f3b9515ea)

Change-Id: Ie40007184f6194ba032b4213c18d36254e80aaa6
Reviewed-on: https://boringssl-review.googlesource.com/1432
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 18:12:41 +00:00
David Benjamin
51b1f7427b Make init_msg a uint8_t*.
It's current a void* and gets explicitly cast everywhere. Make it a uint8_t and
only add the casts when converting it come init_buf, which internally stores a
char*.

Change-Id: I28bed129e46ed37ee1ce378d5c3bd0738fc1177f
Reviewed-on: https://boringssl-review.googlesource.com/1163
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:43:20 +00:00
David Benjamin
13ab3e3ce1 Remove heartbeat extension.
Change-Id: I0273a31e49c5367b89b9899553e3ebe13ec50687
Reviewed-on: https://boringssl-review.googlesource.com/1050
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:48:19 +00:00
David Benjamin
3f6fa3db62 Remove more remnants of compression.
Change-Id: I721914594fc92a66d95c7ec2088f13b68e964103
2014-06-24 18:43:57 -04:00
Adam Langley
e044fe4bc7 Fix null pointer errors.
PR#3394

(Imported from upstream's cea5a1d5f255a6a186cd7944c4a312612da965f3)
2014-06-20 13:17:42 -07:00
Adam Langley
bed2214b3e Fix for CVE-2014-0195
A buffer overrun attack can be triggered by sending invalid DTLS fragments
to an OpenSSL DTLS client or server. This is potentially exploitable to
run arbitrary code on a vulnerable client or server.

Fixed by adding consistency check for DTLS fragments.

Thanks to Jüri Aedla for reporting this issue.

(Imported from upstream's eb6508d50c9a314b88ac155bd378cbd79a117c92)
2014-06-20 13:17:41 -07:00
Adam Langley
895780572b Fix CVE-2014-0221
Unnecessary recursion when receiving a DTLS hello request can be used to
crash a DTLS client. Fixed by handling DTLS hello request without
recursion.

Thanks to Imre Rad (Search-Lab Ltd.) for discovering this issue.

(Imported from upstream's 8942b92c7cb5fa144bd79b7607b459d0b777164c)
2014-06-20 13:17:41 -07:00
Adam Langley
f10a63b050 Typo: set i to -1 before goto.
PR#3302

(Imported from upstream's 646886682373e76dee233f7b918dec0c83e180fc)
2014-06-20 13:17:41 -07:00
Adam Langley
56475207be Add heartbeat extension bounds check.
A missing bounds check in the handling of the TLS heartbeat extension
can be used to reveal up to 64k of memory to a connected client or
server.

Thanks for Neel Mehta of Google Security for discovering this bug and to
Adam Langley <agl@chromium.org> and Bodo Moeller <bmoeller@acm.org> for
preparing the fix (CVE-2014-0160)

(Imported from upstream's 7e840163c06c7692b796a93e3fa85a93136adbb2)
2014-06-20 13:17:39 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00