Date: Tue, 29 Jan 2013 08:30:59 +0200 From: Milen Rangelov <gat3way@...il.com> To: john-dev@...ts.openwall.com Subject: Re: dmg2john Hm I was busy with doing kwallet stuff on GPU yesterday, so I didn't have time to check what's going on. I checked this morning and it looks like my dmg code does not have the ceil() stuff and is using size_t (yet reads and lseeks are unchecked which is another less serious problem). I also had a spurious warning problem that I fixed yesterday when the dmg discussion started and I had to do a benchmark. This spurious warning is not present in dmg2john. I don't know how this was introduced, I guess when we were working on dmg, we were using some old quick and dirty code. There is another potential problem (I don't care about it since I support only x86/x86_64, but it might be problematic for jtr) - there is a htonll() macro which swaps endianness for 64-bit ints. It assumes the machine is little-endian. You need to fix that for JtR, otherwise it would break on big-endian hosts. Regards, Milen On Tue, Jan 29, 2013 at 5:05 AM, Dhiru Kholia <dhiru.kholia@...il.com>wrote: > On Tue, Jan 29, 2013 at 2:39 AM, Solar Designer <solar@...nwall.com> > wrote: > > dmg2john is in bad shape now. Here are some issues: > > > > 1. It's not being built by default. "make dmg2john" builds it, but this > > should be made the default. > > > > 2. It's not integrated into "john", to be similar with other *2john > > tools. It becomes a separate binary executable. Perhaps we need to > > integrate it, since it has no dependencies on extra libs. > > These two issues have been fixed already. I sent a pull request before > I went to bed yesterday. > > > 3. The return values from lseek() are not checked. They must be! > > > > 4. The return values from read() are either not checked or are checked > > incorrectly. "<= 0" is not it. read() may also return with partial > > data. We need to use a read_loop() function (see popa3d), or at the > > very least detect the partial reads and refuse to work if so. > > Alternatively, we may switch to using "FILE *" and the f*() functions. > > > > 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? > > I am looking into these issues. > > -- > Dhiru > Content of type "text/html" skipped
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.