[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 09 Oct 2013 13:40:45 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: 7z valid()
On 2013-10-09 11:51, Dhiru Kholia wrote:
> On 10/09/13 at 12:00am, magnum wrote:
>> 7z is nowhere near Dhiru's high score in "bwtdt" from two months ago though.
>
> It is a work of art, my friend ;(
>
>> I'll spare you the details, although it was so bad I found it funny. It did
>> waste some of my time though, again.
>
> My bad. I usually try to fix them all on a boring day.
The problem with bwtdt was that it passed ANYTHING (well, except "*") as
valid so most any input autodetected as bwtdt and then segfaulted in its
binary() or salt(). That's nothing you can postpone - you just can't
commit such things.
>> Maybe I should enable the "valid killer self-test" by default, even though
>> it usually segfaults instead of fails with a clue. Heck, I could just throw
>> any segfaulting format into unused without further notice, for Dhiru to fix.
>> But even that will waste a lot of MY time and not Dhiru's. Just as compiling
>> the above list did. And just as opening 10 GitHub issues will.
>
> Please turn on this ""valid killer self-test" (if we actually have such a
> thing).
It was active in DEBUG builds. I tried it now but it doesn't seem to
work anymore. It did not segfault nor catch any of the bad ones. Not
sure why - it used to work well.
I changed it a bit and caught a bunch of formats. The openssl-enc now
fails self-test but is left in the tree. The following formats segfaults
so are moved to unused: Haval, MD2, keyring-opencl, Panama, RipeMD,
Skein, Snefru and Tiger. Coincidentally, all have the same author ;-)
Hint: You can't rely on just checking string lengths. If a field is
supposed to be hex, you must check it for *being* hex. And so on (this
obviously includes base 10 numbers that you later read with eg. atoi() -
they must be numbers, and they must be within proper range).
Hint 2: You start every new format by copying an old one - which is
natural. Concentrate on ONE valid() and try to make it as simple and
beautyful as possible yet absolutely unbreakable - look at it as an
exercise, pretend you are writing an Internet daemon running as root,
that will be attacked by millions of script kiddies with fuzzers. I'm
sure you can make it better than the best one I've come up with so far.
Do it *really* good for a single format and then base all future formats
on it. Next time hopefully you can lecture me ;-)
Look at how RAKP now checks for actual hex digits in the fields, and how
AIX checks for Base64. That is (hopefully o.O) foolproof yet it's simple
and readable code. Also, if eg. only lower-case hex digits are allowed,
that is easily accounted for just by changing the HEXCHARS macro
accordingly. BTW, note how these formats also avoid using strtok() nor
allocate any temporary buffer.
This self-test (right now enabled regardless of DEBUG) is temporary,
it's a hack we can't leave active in a release. And it might very well
seg-fault on other formats not caught on my gear right now. Also, it
obviously does not catch all cases of sloppy checks.
magnum
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
