-
Notifications
You must be signed in to change notification settings - Fork 398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segmentation fault on -m 94 -m 95 #5
Comments
What is the RAM capacity of your machine? We observe that Argon2 behaves in a strange way when the requested memory exceeds the physical amount of RAM, but have not reproduced the bug on our machines yet. |
512MB on x86 On OSX with 16GB RAM and x86_64 it works fine:
|
There is a problem with the memory allocation on 32-bit platforms: |
I added a check for integer overflow in commit b7e5cc7. |
Please check if it works now. |
@floyd-fuh could you test the latest version? |
Let's say it works, but it's not really obvious what happens for each invocation... (Btw. as a side note, look at the first invocation I did, that's why having a clean interface from the beginning is so important and you might want to reconsider the decision for using positional arguments for pwd and salt, maybe you just want to rather have arguments -p and -s that are mandatory, but this is off-topic for this bug)
I don't have a clue what's happening with -m 87, but it just takes very long. |
Thanks, there's definitely something going wrong here. Problem is at https://github.com/P-H-C/phc-winner-argon2/blob/master/src/main.c#L348, and due to |
On the 32-bit machine the maximum requested memory is 2 GiB, which is delivered by -m 21 and -m 87 (and all other values equal to 21 mod 22). I suppose that 2 GiB is still too much for your machine. |
then maybe argon2 should just print an error message and gracefully exit? |
It should gracefully exit in case of such errors but I can not reproduce where it fails at your side. I will try to add more error message printing anyway. |
It's just a Virtualbox 32bit machine with an installed Xubuntu image you can assign as much memory as you like for every start of the machine. Additionally you could install AFL as mentioned above on the same box, which can restrict memory (-m switch). Of course you can also use other Linux features to restrict memory of the argon2 process. Additionally, I guess at one point you should seriously build continuous testing with AFL or another fuzzer of your choice. It will catch bugs early in the development process, like the segmentation fault we were talking about here in this issue report earlier on. It will increase security and usability and maybe most important for you: credibility in the security community for testing your code properly. In your case it is even incredibly simple, I gave you a step by step procedure above. In other words: If a memory corruption bug like this shows up in 10 years when people might be using argon2 you will regret that you didn't use a server CPU idling around somewhere or didn't take the time to setup a Virtualbox. In fact I didn't run the fuzzer again on your committed fixes. Of course you might have other things on the todo list at the moment. Let me know if you already tried those approaches to reproduce the error and I'll be happy to help figuring it out further |
You're right, we'll be working n that. |
The most recent commit should be more resilient to such things. |
Closing this one, thanks a lot @floyd-fuh for the testing, please let us know any other issue :) |
So I've run fuzzing tests with American Fuzzy Lop (AFL, http://lcamtuf.coredump.cx/afl/ ), as argon2 takes all it's arguments from command line I used the argv fuzzer included in AFL (in experimental/argv_fuzzing/argv-fuzz-inl.h).
There are only two changes necessary to make argon2 fuzzable by AFL:
AFL_INIT_SET0("argon2");
This means argon2 will read arguments from stdin as zero byte delimited values. See argv-fuzz-inl.h for details. You need to produce test corpus data first (input files where AFL can start from). I only started with one file "test" (where \x00 are zero bytes):
To see if it that test file is correctly working with (reading zero byte delimited values from stdin):
Then AFL can be started with:
afl-fuzz -i /opt/argon2/input -o /opt/argon2/output-001 -M argon2-001 /opt/argon2/argon2-afl/argon2
Something like this should start:
I highly recommend you to set up your own AFL fuzzing instance (like a continuous testing environment). I'm not going to fuzz this project any further for now.
Now let's talk about the results. Turns out that argon2 segfaults on -m 94 or -m 95, but seem to wrap around (integer overflow?) when larger numbers are used (tested on 32bit and ARM):
Should be easy to reproduce. Here's a backtrace:
ASAN (address sanitizer) doesn't seem to catch the bug.
I know that an attacker controlling the m_cost of argon2 is probably a bad idea anyway and shouldn't happen, however, I do expect such a crypto tool to be rock solid.
Moreover I got various crashes that are not reproducible with vanilla argon2, but are probably poping up because of the memory restrictions that AFL enforces (for performance reasons, 25MB here, you can change that with -m to afl-fuzz). While I can't confirm this is an issue it might be worth checking how argon2 performs in memory limited environments.
The text was updated successfully, but these errors were encountered: