Hacker News Reader

Comment by Cyph0n | Hacker News Reader

There is a great comment linked at the bottom of the article that explains what exactly caused the authentication bug (it was strncmp!). Is there somehow a law that states that the higher the severity of a software bug, the simpler the reason behind it?

View on HN

From the comment:

What the programmer should have done is check if the hash coming from the browser has the correct length, 32 characters, before attempting to compare the two strings. Or even better, the programmer should have used the proper string comparing function, strcmp, that already does that for you...

strcmp has its own issues. What they actually should have done is use a cryptography library with properly implemented comparison, because it is surprisingly easy to get wrong.

Also, fixed length hashes aren't strings. They shouldn't use the same functions as strings.

Edit: Using a proper cryptography library also might have saved them from using MD5. At the very least, most modern libraries have a warning that is is deprecated and should not be used for new projects.

Yep, even if they had used memcmp/strncmp correctly the AMT would still be vulnerable to a timing attack here, which is probably even easy, because it runs on a low-power system with, I assume, not much background activity.

In addition to timing attacks, if the attacker supplied a non null terminated string, it could potentially cause a memory overrun. This may not be possible depending on how the string is handled on ingress, but if it is, it would likely have serious consequences.

The attacker supplies ""-enclosed string and the HTTP parser is supposed to verify this.

Maybe they used some in-place HTTP parser and that's why they didn't have zero-termination to use strcmp.

And btw, it's not clear if the code was C to begin with. I believe large chunk of ME firmware is said to be written in Java, which kinda makes sense for a network facing system considering how shitty string.h is.

It would be interesting if somebody found that they are using some COTS webserver, C/Java - doesn't matter, and see if there are some CVEs for it.

I doubt using Java would really help in this case, because in order to use Java they would have to port the entire JVM to whatever platform they are using. Doing this without introducing any security bugs is about as difficult as just doing everything in C to begin with. By the way, the fact that the issue was with strncmp very strongly suggests that the issue was in C code.

Some security paper showed ARM disassembly of the issue, although I'm not sure if that was this specific issue or just another "strncmp-induced" issue.

(Btw. there are many smaller Java VMs for stuff like Java Card and JEFF)

They use ARC architecture and some embedded JVM. Of course this doesn't guarantee that the AMT web server runs on this JVM.

Few years ago somebody published analysis of the ME firmware for servers (desktop version was compressed with unknown algorithm). I'm sure you can google this for details.

> the fact that the issue was with strncmp

But we don't know for sure if it was strncmp. It's a logic bug, which can be implemented in any language. But again, maybe this server runs outside JVM and is written in C.

> Also, fixed length hashes aren't strings.

In HTTP everything is a string.

> Using a proper cryptography library also might have saved them from using MD5.

HTTP, again. Browser sends MD5 whether you like it or not.

> In HTTP everything is a string.

Once you parse it out of HTTP it is no longer as string (or at least it should no longer be a string).

>HTTP, again. Browser sends MD5 whether you like it or not.

They are quite clearly doing verification themselves. They can use whatever algorithm they want.

It has to be MD5. They use HTTP digest auth, which uses MD5. You send a nonce, the browser throws a popup asking for password and returns md5(nonce+pass). No way around it.

Very easy way around it: do NOT use digest auth.

Sure, which brings us to the second point: HTTP's MD5 digest auth is still good enough to keep passive eavesdroppers from learning the password when TLS isn't used, which is its only purpose.

They were implementing HTTP Digest[0]. This leaves no room to choose something other than MD5. And I'm not sure what other HTTP authentication scheme they could had chose for firmware (ie. cannot take dependency on anything in infrastructure, like PKI or Kerb)

[0] https://en.wikipedia.org/wiki/Digest_access_authentication

I think the programmer should have supplied the length of the "computed_hash" not the "response" which as I understood supplied by the user. Like this : strncmp(computed_hash, response, computed_hash_length)

You don't want to use strncmp() for this; aside from the timing attacks it opens up, using strncmp() for these kinds of comparisons implies that the operation you are performing is "string a is a prefix of string b" (or vice-versa).

Even though your example ends up being ok-ish (if the computed hash is a prefix of the response, perhaps it is ok to ignore any trailing junk in the response), intent is important for code quality and maintainability.

In this instance, the operation desired is "string a matches string b", which means strcmp() would be the right solution (ignoring timing attacks).

Of course, since we're talking about sensitive crypto operations here, neither is really the right answer. But in non-crypto contexts, if you want to know if two (valid) strings are the same, just use strcmp().

The "n" and the length argument doesn't automatically make strncmp() "safer" somehow; it is a totally different operation.

Yes. It was hilarious that the poster thought strcmp was the solution...

Though perhaps memcmp with fixed size buffers would be better still, no worrying about null terminated strings.

It almost smells like an obfuscated tactic to evade review. In a rush a reviewer might see strncmp and think "Not using strcmp. Check." without going all the way to consider the strlen as argument, which really should have been a constant and the smoking gun.

That comment id great, but where is the source? I can't find any technical details following links in the article. Moreover, Embedi team who found the vulnerability says[0] on their site: "Intel representatives have asked Embedi to hold off on disclosing any technical details regarding this issue until further notice."

Or is the commenter just guessing?

0. https://www.embedi.com/news/mythbusters-cve-2017-5689

Yes, that is crazy. Checking for match based on the length of the input. If input is "", that matches anything.

Reply on news.ycombinator.com