Live data from Hacker News

Why did the OpenSSL punycode vulnerability happen?

words.filippo.io

51–60 of 104 posts

Re: Why did the OpenSSL punycode vulnerability happen?

#51

Earlier quoted context omitted.

> Not involved in OpenSSL, but this is a fairly common pattern in a lot of C APIs. A better approach IMHO, typically found in the Win32 API for example, is to take an extra argument which receives the buffer size needed to hold the whole output, in addition to the size of the buffer you pass. This allows the caller to detect the condition and decide if it's an error or not if the required buffer size returned is grea…

Isn't that roughly how heartbleed works? Trusting the provided length?

If the caller is an external actor, yes, their provided length should not be trusted. However, this is not always the case. The caller may be another part of the same program, trusted not to perform malicious actions to the same extent that the rest of the program is trusted.

Re: Why did the OpenSSL punycode vulnerability happen?

#52
post #29

> There is a function, ossl_punycode_decode that decodes Punycode. Punycode is a way to encode Unicode as ASCII, used to represent Unicode strings in the ASCII-only world of DNS. ossl_punycode_decode takes an output buffer, and if the buffer runs out it keeps parsing and verifying the Punycode but discards the rest of the output. > I did not have the heart to figure out why it works like this. Maybe there's a good re…

I am by no means a rust developer and asking in ignorance, would this have been a problem had it been written in rust?

> would this have been a problem had it been written in rust?

The answer would be "it depends on whether you consider denial-of-service a problem". The key detail which makes all the difference is that, unless you're playing with raw pointers (which can only be dereferenced in "unsafe" blocks), the pointer to a buffer slice is always kept together with its length (in a "fat pointer"). Attempting to write through it past the buffer's bounds will result in a Rust panic, which usually aborts the whole process (there are ways to abort just one thread, or even to treat it similarly to a C++ exception, but a library cannot depend on them since the program might be compiled in the panic=abort mode). While that's obviously better than allowing for remote code execution, it still could be considered an issue.

Of course, that's assuming you want a similar API and are writing the code in a similar style, just replacing the direct pointer manipulation with slice manipulation. I don't know whether, in this particular case, more idiomatic Rust code would have avoided the issue. And, of course, the "growable container" approach would completely avoid it, but Rust is also used in places where memory allocation is not allowed, so having a non-allocating API still makes sense.

Re: Why did the OpenSSL punycode vulnerability happen?

#53

Earlier quoted context omitted.

But why? where does the complexity come from

parsers and serializers have one thing they often do: read (and write) to a (usually manually allocated) byte array. And the content of that byte array is often under attacker control. C has terrible support for things dealing with byte arrays. They must be manually allocated, and accesses must be checked to be in-bound manually. Lots of critical software have parsers written in C. This combination leads to CVEs like…

> or a NullPointerException in Java

Just nitpicking, but the exception for an invalid array access in Java would be IndexOutOfBoundsException (or one of its subclasses), not NullPointerException.

Re: Why did the OpenSSL punycode vulnerability happen?

#54
post #42

> There is a function, ossl_punycode_decode that decodes Punycode. Punycode is a way to encode Unicode as ASCII, used to represent Unicode strings in the ASCII-only world of DNS. ossl_punycode_decode takes an output buffer, and if the buffer runs out it keeps parsing and verifying the Punycode but discards the rest of the output. > I did not have the heart to figure out why it works like this. Maybe there's a good re…

Even in C, you can write an abstraction for a growable buffer. The problem is, you have to make all the rest of the code work with it, rather than a char* or whatnot.

Then you have the new problem of restricting its growth and that adds a new failure mode to deal with. The C idiom works fine if you don't have implementation bugs.

Re: Why did the OpenSSL punycode vulnerability happen?

#55
post #45

So the blog mentions that in certain cases you have to decode the punycode from one field to compare it to the value in another field. Would it have been safer to encode the data in the other field to punycode and compare the encoded values? That way a hacker can’t mess with your decoder (where bugs like to lie). But at the other end you risk that your encoder has an issue. I do t know how to judge if those are equal…

Encoding and decoding are both transforms on attacker supplied input.

I don't think its helpful to armchair quarterbacking other peoples mistakes that ended up in a vulnerability when the overall code quality is good. There are a dozen ways they could have done it differently, which may or may not have resulted in different exotic bugs.

Punycode was the footgun here. Not the language, or the implementation, or the code. They were forced to do something stupid and complex and dangerous deep within the bowels of a critical library.

Re: Why did the OpenSSL punycode vulnerability happen?

#57

Earlier quoted context omitted.

tl;dr: Using C/C++ and being human => memory safety problems.

Also, as we learned back when Heartbleed was discovered, the OpenSSL code is not in good shape. It "suffers from maintenance", as one clever wag said about legacy code. There's a reason LibreSSL forked the code. More distributions need to switch away from OpenSSL. And before anyone pipes up, I'm not claiming LibreSSL does not and will not ever haver vulnerabilities. I'm saying that ripping stuff like punycode out of…

>Also, as we learned back when Heartbleed was discovered, the OpenSSL code is not in good shape. It "suffers from maintenance", as one clever wag said about legacy code. There's a reason LibreSSL forked the code. More distributions need to switch away from OpenSSL.

Anyone who's ever worked with the OpenSSL API or looked at its code can tell you that it's a steaming pile of crap. It's no surprise that this vulnerability was discovered. Honestly, OpenSSL should just be banned because it's so horrible, and there are better alternatives available.

Re: Why did the OpenSSL punycode vulnerability happen?

#58
post #42

Earlier quoted context omitted.

Even in C, you can write an abstraction for a growable buffer. The problem is, you have to make all the rest of the code work with it, rather than a char* or whatnot.

Then you have the new problem of restricting its growth and that adds a new failure mode to deal with. The C idiom works fine if you don't have implementation bugs.

Everything has that failure mode, in every language.

Re: Why did the OpenSSL punycode vulnerability happen?

#59

Earlier quoted context omitted.

> Not involved in OpenSSL, but this is a fairly common pattern in a lot of C APIs. A better approach IMHO, typically found in the Win32 API for example, is to take an extra argument which receives the buffer size needed to hold the whole output, in addition to the size of the buffer you pass. This allows the caller to detect the condition and decide if it's an error or not if the required buffer size returned is grea…

Isn't that roughly how heartbleed works? Trusting the provided length?

Hearbleed was due to failure to sanitize external data[1], not sure how that's directly relevant to what I wrote.

I mean sure if the caller fails to sanitize and as a result passes a size that's bigger than the actual buffer, the callee doesn't have much in the way of detecting that. But that's a general C issue, nothing specific to what I wrote.

[1]: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h...

Re: Why did the OpenSSL punycode vulnerability happen?

#60
post #41
post #29

Earlier quoted context omitted.

I am by no means a rust developer and asking in ignorance, would this have been a problem had it been written in rust?

In no other mainstream language other than C (and maybe non-idiomatic C++) would this have ever been a problem.

Yeah, most folks in C++ would just use an std::vector, right? I guess if you wanted to use the modern abstraction of std::array, it could be a problem, since using `operator[]` on a non-existent element produces UB.
Post reply on HN