Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 29 Jan 2013 23:15:33 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: dmg2john

On 29 Jan, 2013, at 22:37 , magnum <john.magnum@...hmail.com> wrote:

> On 29 Jan, 2013, at 2:04 , Solar Designer <solar@...nwall.com> wrote:
>> On Tue, Jan 29, 2013 at 01:28:57AM +0400, Solar Designer wrote:
>>> On Mon, Jan 28, 2013 at 11:22:24PM +0200, Milen Rangelov wrote:
>>>> 		cno = ceil(header2.datasize / 4096.0) - 2;
>>>> 		chunk = (unsigned char *) malloc(header2.datasize);
>>>> 		data_size = header2.datasize - cno * 4096;
>>>> 		if (data_size < 0) {
>>> 
>>> So I suggested in the Twitter thread that folks try size_t for now, but
>>> the correct fix would be different, so that the sanity check is not
>>> removed.  Perhaps use ssize_t or "long long", or rewrite the check.
>> 
>> I chose to post a different patch in response to Jeremiah's message on
>> john-users.  That's because there's also a printf format string that
>> uses "%d", and cno and data_size are also of type int in dmg_fmt_plug.c.
>> 
>> The patch that I posted should be good for up to 8 TB.
> 
> Apparently not. I can reproduce his problems. This is with the patch applied:
> 
> Program received signal EXC_BAD_ACCESS, Could not access memory.
> Reason: KERN_INVALID_ADDRESS at address: 0x00000000df7c6000
> 0x000000010006b4ac in print_hex (str=0xdf7c6000 <Address 0xdf7c6000 out of bounds>, len=8192) at dmg2john.c:75
> 75			printf("%02x", str[i]);
> (gdb) bt
> #0  0x000000010006b4ac in print_hex (str=0xdf7c6000 <Address 0xdf7c6000 out of bounds>, len=8192) at dmg2john.c:75
> #1  0x000000010006b9d5 in hash_plugin_parse_hash (filename=0x7fff5fbffc9c "../../test.dmg") at dmg2john.c:164
> (gdb) q
> 
> Line 164:  print_hex(chunk + cno * 4096, data_size);
> 
> Not sure if I should try to grok this and come up with a new temporary patch for Jeremiah or just wait for fundamentally better code?

I sent a patch to Jeremiah for trying out. It just adds this (to be used with Solar's patch as well):

@@ -161,7 +161,7 @@ static void hash_plugin_parse_hash(char *filename)
 		printf("*%d*", header2.encrypted_keyblob_size);
 		print_hex(header2.encrypted_keyblob, header2.encrypted_keyblob_size);
 		printf("*%d*%d*", cno, data_size);
-		print_hex(chunk + cno * 4096, data_size);
+		print_hex(chunk + (long)cno * 4096, data_size);
 		printf("*1*");
 		print_hex(chunk, 4096);
 		printf("\n");

This works for me, except the output is mostly zeros and John can't crack it. Maybe that is the other bug mentioned that I see now?

We should review all 2john programs for similar bugs. I changed a couple of ints to size_t in rar2john half a year ago, it could only handle files smaller than 2 GB before that.

magnum

Powered by blists - more mailing lists

Your e-mail address:

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