15 comments

[ 4.4 ms ] story [ 51.8 ms ] thread
Using the writeup I managed to find a crashing SSH-key which would give `ssh-keygen -l -f $file` a segfault.

Great writeup, and a great tool.

Bug reported to Debian, where it languishes:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=801530

Reported upstream, and seem to be fixed somewhere between OpenSSH 6.6 and 6.8:

http://lists.mindrot.org/pipermail/openssh-unix-dev/2015-Oct...

Brief background on the reason for searching:

http://blog.steve.org.uk/so_about_that_idea_of_using_ssh_key...

List of bugs identified using AFL is available on their webpage[1], seem to have it bookmarked from the last time AFL would have been mentioned here. Always find fuzzing to be a pretty interesting (and impressive) technique.

[1] http://lcamtuf.coredump.cx/afl/#bugs

I found this ctype function bug^Wundefined behavior for glibc with afl-buzz:

https://sourceware.org/bugzilla/show_bug.cgi?id=17674

I was expecting that it would get closed as "undefined behavior" although the "This is perfectly valid undefined behaviour" line made my day.

I just wanted to have this bug documented for other people or in case this behavior would ever be part of a security related issue.

Bogus results is one thing; segmentation faults are another matter entirely! This is especially true for a ctype-type function. "isblank"? I can't even imagine what it would be doing internally for it to be possible to make it crash.

[this post was previously just as above, but in a more allusive form, so apologies if you saw that instead]

From what I remember when trying to understand what was going on, it was a really convoluted, pre-computed array where the parameter to each ctype function was used as an index.

My guess to why they did it this way was that it was an optimization and/or possible support for different locales.

Even though it's undefined behavior, considering that other libc implementations don't have this issue, making a simple function like isblank so complicated that it may crash, makes you wonder what other abominations lurk in that code.

The comment here suggests why: http://osxr.org/glibc/source/ctype/ctype.h

I'd considered a table-driven approach - I mean, perhaps it's a bit daft for isblank, but OK for some of the more random categories - but assumed they'd index on c&0xFF or something, rather than just using the int input verbatim!

It's not immediately obvious to me why that wasn't done (maybe glibc has to support one's complement?), since none of the high-bit ASCII chars appear to have a defined value. Then you can decide EOF is -1 - no reason not to - and it would capture that as well. (The functions don't appear to have a defined output for EOF, even though it's a defined input, so it could just shared the same output as (signed char)-1, and that would be OK. At least it's not a crash.)

The documentation on cppreference is three sentences long, and the invalid inputs get their own line. It's quite clearly documented.

cppreference is a wiki, and that undefined behaviour note was added in 2013 by a user. If you want to help others avoid mistakes by creating documentation, that's a good example of how to do it.

http://en.cppreference.com/w/c/string/byte/isblank

cppreference is documenting the standards and hasn't lots of information about specific implementations.

Technically, this issue is already covered by saying "The behavior is undefined if the value of ch is not representable as unsigned char and is not equal to EOF."

But in reality it is of course hilarious if a simple function like isblank() can crash if though it only check (in the default locale) if its parameter is equal to 9 or 32.

I also checked other libc implementations (IIRC the BSD, OSX, dietlibc and Bionic) and none of the others did crash or even produce surprising results with values outside of the defined range.

Why the hell would you even report something as a bug when the code that causes the crash has undefined behavior? Why do you expect UB to work? Seriously… just… WHY?
Just because it's undefined behavior doesn't forbid you to think about what to do outside of the defined range. Especially if your competitors have done just that.

In this case, we even have a notorious range. EOF (which usually is implemented as -1) and unsigned char. You can't easily represent that in standard C types without using a type that lets most of the type's range fall into the range of undefined behavior.

I didn't expect undefined behavior to work, I didn't know that it was undefined behavior as the parameter you have to pass is an int. So the compiler can't inform me that I'm doing it wrong and at runtime, there are no checks. So, this bug in the code can go unnoticed for a long time.

BTW, note that isascii() is defined on all int values. It's all POSIX, so there is probably a historical reason for that. But the defined range of isascii() proves that covering the whole range of the input parameter's type is not something outrageous to ask for.

Has anyone ever fuzzed fuzzy lop itself using fuzzy lop?
Yes. Didn't find anything.

(Strictly speaking you can't fuzz it itself, because you need something that takes a file input as an input. But there are tools sipped with it, e.g. one that parses the fuzzer's output stats, that you can fuzz.)

Here's a CppCon 2015 presentation that I enjoyed, for whatever that's worth, giving an introductory overview of this sort of technology, with some examples. AFL is mentioned: Kostya Serebryany - Beyond Sanitizers - https://www.youtube.com/watch?v=qTkYDA0En6U