Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160716164924.GA19227@openwall.com>
Date: Sat, 16 Jul 2016 19:49:24 +0300
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: passwdqc code quality

On Sat, Jul 16, 2016 at 07:27:19PM +0300, Solar Designer wrote:
> On Sat, Jul 16, 2016 at 06:40:21PM +0300, Solar Designer wrote:
> > https://bugs.debian.org/831356
> > 
> > The problem is that when we modified passwdqc_check() to use pw_dir in
> > 2009, we didn't similarly update pam_passwdqc.c to set pw_dir in fake_pw
> > for when the "non-unix" option is used.  Looking at it another way, a
> > problem was that we didn't fully initialize fake_pw right away, just in
> > case of future code changes like this.  I'll fix both of these shortly.
> 
> As a test, I added just:
> 
>                 memset(pw, 0, sizeof(*pw));
> 
> right after "pw = &fake_pw;"
[...]
> Without the memset(), the issue was not detected by ASan.  So it looks
> like ASan is of little help for issues like this as the segfault would
> be immediately visible without it anyway, when the segfault does occur,
> whereas in our default build on Owl it did not.

I should add: I think UBSan should ideally have detected the issue, and
ASan is not particularly relevant (except to ease debugging).

UBSan was also enabled in this build, with the "-fsanitize=undefined"
option passed during compilation of all source files and during linking.
I guess it currently doesn't work across translation file boundaries.
Our missed initialization is in one source file and our use of the
pointer is in another source file.  There are probably other limitations
as well.

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.