Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 22 Mar 2015 00:40:48 +0300
From: Alexander Cherepanov <ch3root@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Extend AFL to fuzz as you want

On 2015-03-19 06:13, magnum wrote:
> On 2015-03-19 02:21, Alexander Cherepanov wrote:
>> On 2015-03-18 15:35, Frank Dittrich wrote:
>>> On 03/18/2015 01:12 PM, Alexander Cherepanov wrote:
>>>> I think this is a general question to be discussed in john-dev, not
>>>> limited by the needs of fuzzing or security in general. Talking
>>>> specifically about fuzzing, when you want to fuzz functions behind the
>>>> valid() it's easier to patch this specific check out of valid() for now.
>>>
>>> But isn't the purpose of valid() to make sure all the other format
>>> methods only have to work with sane/sanitized input?
>>> Why should we care about segfaults etc. that would only occur after you
>>> removed some of the sanity checks in valid?
>>
>> That's an interesting question. Short answer: we should care about it
>> because such a crash could be due to a genuine bug. But it very much
>> depends on a particular hash/valid()/get_salt()/etc.
>
> Knock yourselves out guys, just do not open GitHub issues for crashes
> that you can only trigger after removing whatever protection we had in
> place for them. Not unless you actually *did* find a genuine bug *and*
> can describe (or fix) it.

Sure, removing such safety checks is only a trick to make deeper fuzzing 
easier. After something interesting is found one has to check if it's 
applicable to the non-modified program. If a genuine bug is found 
usually it's easy to modify the sample to be acceptable the non-modified 
program.

That was an easy part, now a bit more tricky one:-) There is a crash in 
7z format if you raise LINE_BUFFER_SIZE to, say, 1000000:

$ perl -le '$k = 300000; print q{$7z$0$1$$$$$0$}, $k, q{$0$}, q{1} x ($k 
* 2)' > test.pw
$ ./run/john test.pw
Loaded 1 password hash (7z, 7-Zip [SHA256 AES 32/64])
Will run 4 OpenMP threads
Note: This format may emit false positives, so it will keep trying even 
after
finding a possible candidate.
Press 'q' or Ctrl-C to abort, almost any other key for status
Segmentation fault

Is this a genuine bug in your opinion?

On a related note, I think it would be nice to drop most lengths from 
hashes. An attached patch both fixes the crash and ignores the data 
length recorded in the hash (as a first step in this direction).

-- 
Alexander Cherepanov

>From f8ed7f5e38568fb700f6c4f470b758640bac87ff Mon Sep 17 00:00:00 2001
From: Alexander Cherepanov <ch3root@...nwall.com>
Date: Sun, 22 Mar 2015 00:01:20 +0300
Subject: [PATCH] 7z: compute data length from data and bound it

---
 src/7z_fmt_plug.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/7z_fmt_plug.c b/src/7z_fmt_plug.c
index 78f7a4f..ae40a63 100644
--- a/src/7z_fmt_plug.c
+++ b/src/7z_fmt_plug.c
@@ -195,13 +195,14 @@ static void *get_salt(char *ciphertext)
 	p = strtokm(NULL, "$"); /* crc */
 	cs->crc = atou(p); /* unsigned function */
 	p = strtokm(NULL, "$");
-	cs->length = atoi(p);
+        /* ignore data length taken from the hash */
 	p = strtokm(NULL, "$");
 	cs->unpacksize = atoi(p);
 	p = strtokm(NULL, "$"); /* crc */
-	for (i = 0; i < cs->length; i++)
+	for (i = 0; i < BIG_ENOUGH && p[i * 2] && p[i * 2 + 1]; i++)
 		cs->data[i] = atoi16[ARCH_INDEX(p[i * 2])] * 16
 			+ atoi16[ARCH_INDEX(p[i * 2 + 1])];
+        cs->length = i;
 	MEM_FREE(keeptr);
 	return (void *)cs;
 }
-- 
1.7.10.4


Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ