More Bugs in OpenSSL – DTLS Packet Injection

Another round of vulnerabilities for OpenSSL were published on June 5th, so I ended up spending a chunk of my weekend going over the diffs to make sure they did things right. They have made errors before with some of their fixes.
One of the vulnerabilities peaked my interest:
DTLS invalid fragment vulnerability (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.

There is a great writeup on why the code failed that can be found here.
So, I grabbed the latest and prior versions of the code and loaded up the diffs so I could see the impact.
Now, before I go to far, it is probably important that you familiarize yourself with the blog post “OpenSSL is written by monkeys.
Digging through the code, it easy to sympathize with the monkeys belief. From d1_both.c:
if (item == NULL)
  goto err;
  i = -1; <<===—— ????????????????????????????? }
Yes, the above was fixed but this was in the code base how long???

Anyways, looking at the actual fix, the following message length check was added. From d1_both.c:
unsigned long frag_len = msg_hdr->frag_len, max_len;
  frag = (hm_fragment*) item->data;
  if (frag->msg_header.msg_len != msg_hdr->msg_len) <<==— THIS CHECK WAS ADDED   {     item = NULL;     frag = NULL;     goto err;   } }
Don’t even get me started on the convention used to declare these variables. Yes, this is still in the code! The new check validates that the fragment length of the incoming packet matches the claimed length in the prior fragment.
Digging further into the code we can find some interesting gems:

i = s->method->ssl_read_bytes(s,SSL3_RT_HANDSHAKE, devnull,

Why??? Why would anyone program this??? sizeof(devnull)?? This is the code that monkey legends are made of. Yes, this is still in the code!

As for the exploit, an attacker can inject a DTLS packet fragments (its pretty trivial to inject DTLS packets) into an existing stream with a length larger then prior fragment declared, causing a write of data across the heap. Since OpenSSL uses pointers to functions in structs for overloading all over the place (monkeys?), there is a slight chance it could be exploited to run arbitrary code. I say slight because the attacker would need to figure out where the malloc for the data packet aligned to in the heap; with the way OpenSSL is always allocating memory, and with other incoming packets, this would be hard to exploit. It would be trivial to create a system crash.

The monkeys reference is harsh on the OpenSSL folks, who have been nothing but courteous and helpful in all my interactions but I must note that the same programmer for this bug is also responsible for the Heartbleed commit. At least it made for an interesting weekend...