Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 20 Jul 2016 18:57:43 +0300
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: passwdqc code quality

On Sun, Jul 17, 2016 at 03:21:26AM +0300, Solar Designer wrote:
> 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.

For now, I am going to switch to size_t only for local variables holding
strlen() return value, so that our ABI is preserved.

I am also going to introduce a hard-coded length limit of 100000, after
which REASON_LONG is always returned.  Checking a password this long
takes a second.  Checking a password 10 times longer (1000000 chars)
takes a minute.  (Neither makes much sense.  Normally, "max" would be
set way lower.  By default, it is 40.)

Not to break existing configs (in a minor version update like this), max
up to INT_MAX will still be accepted, but will be capped at 100000
internally.

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.