Blues for SSL
UPDATE 4/11/2014 @ 13:47 Mountain: Matrix has released version 3.6.1 to address the integer underflow bugs I found. Find the update here: http://www.matrixssl.org/news.html
UPDATE 4/09/2014 @ 20:51 Mountain: Matrix has acknowledged the bugs I've found and are formulating the appropriate patches. Stay tuned! These guys are quick! :-)
UPDATE 4/09/2014 @ 20:51 Mountain: Matrix has acknowledged the bugs I've found and are formulating the appropriate patches. Stay tuned! These guys are quick! :-)
UPDATE 4/09/2014 @ 20:06 Mountain: Matrix and I are in contact. Quick response from them!
Even if you've been hiding under a rock for the past week, #HeartBleed has certainly found you. I had no intention to get involved, mostly because I have great respect for Neel Mehta that goes back years before his tenure at Google. Everything that needed to be said about this vulnerability was said. A lot. Too much, really.
People have been flooding the TwitterFaceTumblerNets with screengrabs galore, rife with the champion hexadecimal dump, the solace of the SESSIONID mined from binary blobs. Ah, yes, it was a nightmare and a field day for techies everywhere. But, for guys like me, guys a lot lamer than they used to be, it was a pass-over. The necessary work had been done, and I had no business breaking any "cryptographically stone hearts" or wiping the blood of servers over their door.
But, today, on my way to a finance meeting, it occurred to me that there was one thing left to prove. Embedded systems, to me, posed the greatest risk to Internet stability with respect to Heartbleed. As an Internet of Things enthusiast, I was concerned, and then relieved, by the architectural decisions of the engineers at various top-down IoT firms. It was likely that things were on the up-and-up. But, like I said, there was one thing left to prove.
When I got home, I quickly audited the three most popular SSL implementations for embedded systems. Wolf/Cya, Polar, and MatrixSSL were all on the menu. The feast was short, unfortunately, because none of these implementations even bothered with the TLS Heartbeat extension. I had work to do, so I let these sleeping dogs lie.
Then, I was shot with sudden inspiration: What if the bug had nothing to do with Heartbeat, itself? But, was - as we often see in information security - the larger issue of dealing with integers and memory buffers in the C language. Of course that was the problem! Why did I ever feed into the sensationalist bullshit that it was one single extension.
So, I looked back at the MatrixSSL implementation. What did I see? Right off the bat, integer issues were indeed suspect.
In the example below, the MatrixSSL source code correctly ensures that the size of the message buffer is suitable for the extension and its attribute size. There are two primary locations where this code comes into play. First, the entirety of the Extension buffer is checked at line 1969 in sslDecode.c. Feel free to follow along in the open version of MatrixSSL released earlier this week.
extLen = *c << 8; c++; /* Total length of list */
extLen += *c; c++;
if ((uint32)(end - c) < extLen) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
while (c != end) {
extType = *c << 8; c++; /* Individual hello ext */
extType += *c; c++;
if (end - c < 2) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
extLen = *c << 8; c++; /* length of one extension */
extLen += *c; c++;
if ((uint32)(end - c) < extLen) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
extLen += *c; c++;
if ((uint32)(end - c) < extLen) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
while (c != end) {
extType = *c << 8; c++; /* Individual hello ext */
extType += *c; c++;
if (end - c < 2) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
extLen = *c << 8; c++; /* length of one extension */
extLen += *c; c++;
if ((uint32)(end - c) < extLen) {
ssl->err = SSL_ALERT_DECODE_ERROR;
psTraceInfo("Invalid extension header len\n");
return MATRIXSSL_ERROR;
}
In the above code snippet, we can see the correct evaluation of the extension length. But, what's missing? Ah, yes. It's validating that the extension size doesn't exceed the boundary of the buffer, but it doesn't validate that the size is sensible. In other words, is the value large enough to hold information? That's also an issue that C coders often miss when dealing with buffers.
As a result, the MatrixSSL code is indeed vulnerable to abuse. It is not even remotely similar to the level of importance attributed to the OpenSSL bug. But, this one deserves a high priority. I have reached out to the MatrixSSL support team and will provide them with the details of the attack as soon as they get back to me. When they provide a fix, I'll discuss on this blog exactly what the results of this issue would be.
NOTE FOR CLARITY: There is a lot more to the vulnerabilities I uncovered today than the code snippet above. My job isn't to leak that information, though anyone with a strong C auditing background can recreate what I've found based on the provided data in this post. I'm only publishing this because of the fast and positive communication I've had with the MatrixSSL team. I have no intention to expose the details of the bugs until they are patched.
NOTE FOR CLARITY: There is a lot more to the vulnerabilities I uncovered today than the code snippet above. My job isn't to leak that information, though anyone with a strong C auditing background can recreate what I've found based on the provided data in this post. I'm only publishing this because of the fast and positive communication I've had with the MatrixSSL team. I have no intention to expose the details of the bugs until they are patched.
Best,
D
No comments:
Post a Comment