[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150419101823.GA10719@openwall.com>
Date: Sun, 19 Apr 2015 13:18:23 +0300
From: Aleksey Cherepanov <lyosha@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Improving Johnny
Hi Mathieu,
On Sat, Apr 18, 2015 at 09:03:39PM -0400, Mathieu Laprise wrote:
> >
> > here is a line in Johnny:
> >
> > temp << QString("%1:%2::%3\n").arg(user).arg(hash).arg(hash);
> >
> > Do you see any problems with it? If there is a problem then
> > - provide an example of input to trigger the problem.
> > - How would you fix it?
> > - Does the fix work with your example?
> > - Push the fix into a separate branch in your public repo please.
> >
>
> I really don't see how this question would tell you how experienced I am as
> a software engineer
Of course it is not the only question I look in. And the question is
not only about programming, it is about security too.
If you're interested in security you have to develop critical view.
> so I probably didn't understand it well. Ah ah :( So
> here is just my hypothesis after thinking about it for 2 minutes.
> If we were using unsafe functions like gets in C, a buffer and reading from
> a file, it'd be important to verify that size of user input isn't bigger
> than our buffer but luckily QString and QTextStream seem to be safe against
> buffer overflow.
Ugh, this paragraph makes a sinister ambience. Though it is good to
look at different parts.
> So the problem is that if the user has a %1 in its username, Johnny won't
> write the correct infos to the file and the user will say "This software
> doesn't work!" and we're gonna loose a happy user !
> Correction:
> temp << user << ":" << hash << "::" << hash << '\n';
Great! Really, user and hash are not trusted while such format strings
could not be used with untrusted data. Both user and hash may contain
%1 (%2 and %3 would be bad too). Hash may contain %1 in salt part.
They screw the line and we don't get the result.
1) It's a bug.
2) This bug may be used as a defensive measure against Johnny so IMHO
it may be viewed as a vulnerability.
I wrote about it earlier.
http://www.openwall.com/lists/john-dev/2014/11/07/1
But we did not fix that yet.
Thanks!
--
Regards,
Aleksey Cherepanov
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.
