Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160717002126.GA23042@openwall.com>
Date: Sun, 17 Jul 2016 03:21:26 +0300
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: passwdqc code quality

On Sat, Jul 16, 2016 at 06:40:21PM +0300, Solar Designer wrote:
> This prompted me to check passwdqc code quality in other ways.

We could also want to clean up our use of integer types, but this would
require heavy re-testing.  Right now, we have subtle issues like
"pwqcheck max=2147483647" triggering this:

pwqcheck.c:176:20: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'

This is in:

	int size = 8192;
[...]
	if (params.qc.max + 1 > size)
		size = params.qc.max + 1;

We also use "size + 1" in read_line(), but it uses "unsigned int".

Luckily, our parser is such it doesn't accept values higher than
2147483647, so I think it's only this one value that is problematic now.

Another detail I noticed, by testing with large "max=", is that we clean
buffers to the full allocated size per each password tested when running
with "--multi":

cleanup:
	memset(&pwbuf, 0, sizeof(pwbuf));
	clean(pwline, size);
	clean(oldpass, size);
	clean(newpass, size);

	if (multi)
		goto next_pass;

Perhaps we should clean only to the actually used size, and/or only
perform the cleanup when we're done checking all passwords.  I prefer
the former, since cleaning up only to the actually used size should
usually be fast.

Alexander

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.