[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8B4AD1563B01465D9F19AAB8E0E698D7@ath64dual>
Date: Thu, 9 Jul 2009 21:51:50 -0500
From: "JimF" <jfoug@....net>
To: <john-users@...ts.openwall.com>
Subject: Re: patch for new john format: phpass (also works for phpBBv3)
----- Original Message -----
From: "Solar Designer" <solar@...nwall.com>
To: <john-users@...ts.openwall.com>
Sent: Thursday, July 09, 2009 8:38 PM
Subject: Re: [john-users] patch for new john format: phpass (also works for
phpBBv3)
> No, I was referring to lines like:
>
> + sixbits = (unsigned)strchr(itoa64, *pos++);
> + sixbits -= (unsigned)itoa64;
>
> Here you unnecessarily get into the dark area of casting pointers to
> integers (even to integers of different size on some architectures).
> A more correct way to write this would be:
>
> sixbits = strchr(itoa64, *pos++) - itoa64;
>
> that is, doing pointer math, which is well-defined (the result is of
> signed integer type known as ptrdiff_t on modern systems). This assumes
> that the string had been pre-validated, such that strchr() wouldn't
> return NULL (I think you do have such pre-validation in place).
>
> Of course, changing to atoi64[ARCH_INDEX(...)] is an even better fix.
I see that now, and it is part of the phpass-3.diff. One of the first
things
I did, was to build the phpassmd5_binary() function. It was not well
done, but 'worked' for me. I put a comment there, because I also did not
like it. I did not know how to use atoi64 at that time, and saw examples
'similar' to what I had done, in some other _fmt.c files, so went with that,
but knew it was bad. However, looking for a bug in the wrong place,
did show some other issues (in the non-mmx code within the get_hash_x()
functions. Now those functions are much simpler, with no conditional
compilation blocks.
The phpassmd5_binary is now done with atoi64[ARCH_INDEX()] and
it is much simpler, cleaner and easier to follow. And yes, I do see the
portablity issues in the original code. Worked on my box, but might have
had problems in other places.
>> I now have a phpass-3.diff (but I need to test some). But as we talked
>> about before (off list), I will look at getting this out in the bigger
>> set of changes I have.
>
> No, we have not agreed about anything like that. I merely asked you to
> post to the list about your bigger set of changes.
I did not mean to infer the changes were going to be the way it is, just
that I was going to post, and put them out. I have some additional
clean up to do, then get them into a couple diffs, so that they will
'intermix' and properly patch.
>For now, I'd prefer
> you to release your phpass-3.diff stuff separately, if you don't mind.
> You can also have it as part of your bigger patch, if you need that in
> order to take advantage of other changes that you're making.
--
To unsubscribe, e-mail john-users-unsubscribe@...ts.openwall.com and reply
to the automated confirmation request that will be sent to you.
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.
