[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.
