Bug 20616 - Limit crypt rounds to avoid denial of service
Summary: Limit crypt rounds to avoid denial of service
Status: UNCONFIRMED
Alias: None
Product: glibc
Classification: Unclassified
Component: crypt (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-17 08:07 UTC by Hanno Boeck
Modified: 2020-01-22 13:03 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security+


Attachments
patch to avoid DoS in crypt function (411 bytes, patch)
2016-09-17 08:07 UTC, Hanno Boeck
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hanno Boeck 2016-09-17 08:07:13 UTC
Created attachment 9514 [details]
patch to avoid DoS in crypt function

There are situations where a user can control a password hash. A user could exploit this and cause a DoS by selecting an insanely high rounds value for a password hash.

The current maximum rounds number is 999999999 (a billion minus 1), which would run several minutes on a modern machine. I don't think there is any valid use case for this.

To avoid DoS I would propose to limit the number of rounds to a sane value. It's a bit arbitrary which value to choose, in the attached patch I limit it to 9999999 (10 million minus 1), which runs 3/5 seconds (depending if sha256/512) on my system. It seems like this is probably above all valid use cases, but doesn't DoS too much. Also the original implementation would cap the rounds to 999999999 for all larger values [1], I changed that to return NULL for too large values (to avoid having identical inputs generating different outputs for different glibc versions). Patch attached, changes it for both sha256/512 in the same way.

(P.S.: musl libc already does essentially the same that I'm proposing here [2])

[1] http://people.redhat.com/drepper/sha-crypt.html
[2] http://git.musl-libc.org/cgit/musl/tree/src/crypt/crypt_sha256.c?id=5505f6afae9acf37ef565c68a07ca3df7b1ae2cb#n168
Comment 1 Adhemerval Zanella 2017-01-30 13:40:56 UTC
I think this change is reasonable, the only details I am not sure is which ROUNDS_MAX would the most suitable, mainly on slower CPUs.  On ARMv7 Cortex-A15 using 9999999 rounds it took about 20s, which is high but still manageable.  Can you send this patch upstream?
Comment 2 Hanno Boeck 2017-01-30 13:45:00 UTC
"Can you send this patch upstream?"

What do you mean by this? Isn't this the official glibc bug tracker, thus upstream?
Comment 3 Adhemerval Zanella 2017-01-30 13:55:01 UTC
(In reply to Hanno Boeck from comment #2)
> "Can you send this patch upstream?"
> 
> What do you mean by this? Isn't this the official glibc bug tracker, thus
> upstream?

Patch discussions are not done in bugtracker, but rather on libc-alpha [1]. By send this upstream I mean send a complete patch for libc mailist.

[1] https://www.gnu.org/software/libc/involved.html