Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 16 Sep 2015 15:11:57 -0500
From: jfoug <jfoug@...nwall.net>
To: john-dev@...ts.openwall.com
Subject: Re: ldr_split_line() performance regression


On 9/16/2015 3:03 PM, Solar Designer wrote:
> On Wed, Sep 16, 2015 at 02:02:34PM -0500, jfoug wrote:
>> On 9/16/2015 1:52 PM, Solar Designer wrote:
>>> strlen(*ciphertext) < 10 && strncmp(*ciphertext, "$dummy$", 7)) {
>> These should be reversed, since strncmp should short circuit out much
>> earlier than the length check.  Also, checking for a '$' char even
>> before a strncpy (or even '$' and 'd') would be smart,
> Good catch.  I suggest we do:
>
>          if (((*login)[0] == '+' && (!(*login)[1] || (*login)[1] == '@')) &&
>              (*ciphertext)[0] != '$' &&
>              strlen(*ciphertext) < 10 && strncmp(*ciphertext, "$dummy$", 7)) {
I would still recommend reversing the strlen and strncmp:

         if (((*login)[0] == '+' && (!(*login)[1] || (*login)[1] == '@')) &&
             (*ciphertext)[0] != '$' && (*ciphertext)[1] != 'd' &&
             strncmp(&((*ciphertext)[2]), "ummy$", 5) && strlen(*ciphertext) < 10) {

That code will almost never do the strncmp or strlen, UNLESS it is a dummy.
>> as would checking the 6 byte string "dummy$' from offset 1.
> I'm not sure if it's worth it, and it won't matter for the current
> testcase anyway. ;-)
>
> We could also use strnlen() there, if we depend on it elsewhere in the
> jumbo tree anyway.
>
Fully agree to that.  If it must be used, then do it early, and use the 
results.

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.