Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 19 Apr 2015 13:18:23 +0300
From: Aleksey Cherepanov <>
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.

But we did not fix that yet.


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.