Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 16 Jul 2016 20:59:21 +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:
> As a test, I added just:
> 
>                 memset(pw, 0, sizeof(*pw));
> 
> right after "pw = &fake_pw;"
> 
> Now I am finally able to reproduce the reported issue (with "non-unix"
> in passwdqc.conf):
> 
> ---
> Enter new password: 
> ASAN:DEADLYSIGNAL
> =================================================================
> ==498503==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x2b854b02b510 bp 0x7fffe42b7120 sp 0x7fffe42b68a8 T0)
>     #0 0x2b854b02b50f in __GI_strlen (/lib64/libc.so.6+0x7150f)
>     #1 0x2b8548d335ac in __interceptor_strlen ../../../../libsanitizer/asan/asan_interceptors.cc:577
>     #2 0x2b854ef8067f in unify (/lib64/libpasswdqc.so.0+0x167f)
>     #3 0x2b854ef80f25 in passwdqc_check (/lib64/libpasswdqc.so.0+0x1f25)
>     #4 0x2b854ed739fe in pam_sm_chauthtok (/lib64/security/pam_passwdqc.so+0x59fe)
>     #5 0x2b854a9a70fc in _pam_dispatch (/lib64/libpam.so.0+0x30fc)
>     #6 0x2b854a9ab350 in pam_chauthtok (/lib64/libpam.so.0+0x7350)
>     #7 0x400ea0  (/usr/bin/passwd+0x400ea0)
>     #8 0x2b854afd78f0 in __libc_start_main ../sysdeps/generic/libc-start.c:209
> 
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x7150f) in __GI_strlen
> ==498503==ABORTING
> ---

I made an error in the test above: I forgot to provide the newly built
libpasswdqc.so, so the system's one was used, and thus ASan/UBSan were
not used for passwdqc_check.c, where the pw_dir pointer is used.  With
this corrected, I get an extra message:

passwdqc_check.c:186:29: runtime error: null pointer passed as argument 1, which is declared to never be null

The full output:

---
Enter new password: 
passwdqc_check.c:186:29: runtime error: null pointer passed as argument 1, which is declared to never be null
ASAN:DEADLYSIGNAL
=================================================================
==502004==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x2b7ed4980510 bp 0x7fff9a0b1d40 sp 0x7fff9a0b14c8 T0)
    #0 0x2b7ed498050f in __GI_strlen (/lib64/libc.so.6+0x7150f)
    #1 0x2b7ed24635ac in __interceptor_strlen ../../../../libsanitizer/asan/asan_interceptors.cc:577
    #2 0x2b7ed40dc16c in unify (/home/gcc/passwdqc-1.3.0.1/libpasswdqc.so+0xc16c)
    #3 0x2b7ed40dda38 in passwdqc_check (/home/gcc/passwdqc-1.3.0.1/libpasswdqc.so+0xda38)
    #4 0x2b7ed87739fe in pam_sm_chauthtok (/lib64/security/pam_passwdqc.so+0x59fe)
    #5 0x2b7ed42fc0fc in _pam_dispatch (/lib64/libpam.so.0+0x30fc)
    #6 0x2b7ed4300350 in pam_chauthtok (/lib64/libpam.so.0+0x7350)
    #7 0x400ea0  (/usr/bin/passwd+0x400ea0)
    #8 0x2b7ed492c8f0 in __libc_start_main ../sysdeps/generic/libc-start.c:209

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x7150f) in __GI_strlen
==502004==ABORTING
---

Line 186 is the one with strlen():

static char *unify(char *dst, const char *src)
{
	const char *sptr;
	char *dptr;
	int c;

	if (!dst && !(dst = malloc(strlen(src) + 1)))
		return NULL;

However, this does not catch the underlying issue, and is only a
debugging aid.  If the pointer happens to be NULL, or is indeed forced
to NULL with a memset() like it is in this build, we'll have a crash
anyway, and will know to investigate.

When I remove the memset(), there's no crash on this system, even in
the build with gcc-7-20160710 with ASan and UBSan and with the library
properly loaded this time.

Thus, the conclusion is the same: these tools would have been of no help
to detect the bug.

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.