Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 29 Jan 2013 01:17:06 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: dmg2john

On Tue, Jan 29, 2013 at 01:09:01AM +0400, Solar Designer wrote:
> 5. As also spotted by Milen:
> 
> <@gat3way> @jmgosney @jeremiahg @DhiruKholia @solardiz Hm I think I found the problem....cno = ceil(header2.datasize / 4096.0) - 2; cno is int
> 
> We must not do any floating-point math.  When header2.datasize is large,
> there may be precision loss here, and the resulting value may be other
> than what we expect.  We should express this without resorting to
> floating-point intermediate values:
> 
> 	cno = (header2.datasize + 4095) / 4096 - 2;
> 
> Milen - is this what you meant, too?

Oh, there's more to it.  We do:

		cno = ceil(header2.datasize / 4096.0) - 2;
		chunk = (unsigned char *) malloc(header2.datasize);
		data_size = header2.datasize - cno * 4096;

and we have a nice undefined signed integer overflow in "cno * 4096" and
when assigning to data_size.

These int's should probably be declared size_t instead.

Alexander

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.