However, it was rejected (privately) by two maintainers at the time because it does not come with a proof that it does not have any other vulnerabilities.
Uhm… what? That’s not how security patches work. Or security in general. Because even if it does have a new vulnerability, an attacker now has to spend more time investigating and finding that vulnerability.
Security is merely an economic issue. All we can do is make attacks expensive by constantly fixing bugs. This feels like a huge blow to the haskell ecosystem that claims to care about correctness. I’ve been baffled before by maintainers showing little to no concern about security issues (e.g. TLS/cryptonite packages). This adds to it.
Ugh, how unfortunate. I figured with public disclosure should come a public issue to track discussion… https://github.com/haskell/aeson/issues/864
There’s further discussion which gives some context on the reddit thread: https://www.reddit.com/r/haskell/comments/pm7rcr/cs_syd_json_vulnerability_in_haskells_aeson/hcfzyvt/
In that discussion there’s a pretty good explanation of why the proposed change didn’t really resolve the underlying issues: “Collisions still happen. The only difference is how they’re handled. Currently, with collision arrays. With the proposed PR, with additional HashMap
s. The danger of the latter is that they could potentially degrade to a linked list of singleton HashMaps
, which is even worse than an array.”
I’d add that I think the signposting on this issue itself is unfortunate. The post should specify this is a DOS vulnerability, which is very different than a vulnerability that can lead to privilege escalation, or one which can lead to leaking of data, or etc.
Additionally, as the post itself notes, the fact that hashmaps are vulnerable to collision DOS attacks has been known and discussed for years. If I recall there were a few efforts at changing the Hashable API over time to allow varying mitigations of this, even – see a grep for “salt” in the Hashable changelog: https://hackage.haskell.org/package/hashable-1.3.3.0/changelog
I agree that community attention is good on highlighting this known issue, and I think the reddit thread is converging on a reasonable approach – changing aeson itself to maybe use a better map type, given that it is a library often touching untrusted data. I’m just not fond of discussing vulnerabilities without specifying what sort they are, what the consequences are, and just how novel they are or are not much more upfront than the headline here.
(edit: I should also add that having reviewed the documentation for unordered-containers, and given that we’ve known for a long time that they are vulnerable to decay of asymptotics on untrusted input, that documentation really should highlight what they should and should not be used for.)
Note that this is a flavor of hashdos: https://arstechnica.com/information-technology/2011/12/huge-portions-of-web-vulnerable-to-hashing-denial-of-service-attack/
Many languages took years to fix it, some even never bothered.
This is really disappointing.
The resistance @NorfairKing has gone up against is even more disappointing.
@NorfairKing and team should be praised and thanked!
We as a community need to put pressure on the issue until it is resolved. Let’s not watch it sit and fester longer, we’ll look really silly when this is the root cause of a larger, more catastrophic failure.
I wonder how many payment APIs are using aeson
…?