Bug #9169
closedlibsldap: comparison between pointer and zero character constant
100%
Description
Issue found by gcc 7 build:
../common/ns_crypt.c:101:19: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (++anHexaStr != '\0') { ^~ ../common/ns_crypt.c:101:7: note: did you mean to dereference the pointer? if (++anHexaStr != '\0') { ^~ ../common/ns_writes.c: In function '__s_cvt_hosts': ../common/ns_writes.c:2035:54: error: comparison between pointer and zero character constant [-Werror=pointer-compare] ptr->h_addr_list == NULL || ptr->h_addr_list[0] == '\0') { ^~ ../common/ns_writes.c:2035:34: note: did you mean to dereference the pointer? ptr->h_addr_list == NULL || ptr->h_addr_list[0] == '\0') { ^ ../common/ns_writes.c: In function '__s_cvt_services': ../common/ns_writes.c:2347:61: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (ptr->s_name == NULL || ptr->s_port < 0 || ptr->s_proto == '\0') { ^~ ../common/ns_writes.c:2347:48: note: did you mean to dereference the pointer? if (ptr->s_name == NULL || ptr->s_port < 0 || ptr->s_proto == '\0') { ^ ../common/ns_writes.c: In function '__s_cvt_netmasks': ../common/ns_writes.c:2561:19: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (ptr->netmask != '\0') { ^~ ../common/ns_writes.c:2561:6: note: did you mean to dereference the pointer? if (ptr->netmask != '\0') { ^ ../common/ns_writes.c: In function '__s_cvt_netgroups': ../common/ns_writes.c:2619:16: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (ptr->name != '\0') { ^~ ../common/ns_writes.c:2619:6: note: did you mean to dereference the pointer? if (ptr->name != '\0') { ^ ../common/ns_writes.c: In function '__s_cvt_bootparams': ../common/ns_writes.c:2718:16: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (ptr->name != '\0') { ^~ ../common/ns_writes.c:2718:6: note: did you mean to dereference the pointer? if (ptr->name != '\0') { ^ ../common/ns_writes.c: In function '__s_cvt_ethers': ../common/ns_writes.c:2782:38: error: comparison between pointer and zero character constant [-Werror=pointer-compare] if (ptr->name == NULL || ptr->ether == '\0') { ^~ ../common/ns_writes.c:2782:27: note: did you mean to dereference the pointer? if (ptr->name == NULL || ptr->ether == '\0') { ^ cc1: all warnings being treated as errors
Updated by Joshua M. Clulow almost 4 years ago
Testing Notes¶
First, I set up a test OpenLDAP directory as described here
This directory contained a minimal structure including a proxy account that I could use to bind and search. I was able to check the behaviour of the modified ascii2hex
function:
# dtrace -n 'pid$target::dvalue:entry { printf("dvalue() input: \"%s\"\n", copyinstr(arg0)); } pid$target::ascii2hex:entry { ustack(); self->lenp = args[1]; printf("input: \"%s\"\n", copyinstr(arg0)); } pid$target::ascii2hex:return { printf("lenp %p len %d output:\n", self->lenp, *self->lenp); tracemem(copyin(arg1, *self->lenp), 64, *self->lenp); self->lenp = 0; } pid$target::dvalue:return { printf("dvalue output(): \"%s\"\n", copyinstr(arg1)); }' -c 'ldaplist passwd' dtrace: description 'pid$target::dvalue:entry ' matched 4 probes ldaplist: Object not found dtrace: pid 883 exited with status 1 CPU ID FUNCTION:NAME 1 66924 dvalue:entry dvalue() input: "{NS1}ecc423aad0" 1 66925 ascii2hex:entry libsldap.so.1`ascii2hex libsldap.so.1`dvalue+0x53 libsldap.so.1`__s_api_getDefaultAuth+0x336 libsldap.so.1`getConnection+0x53d libsldap.so.1`__s_api_getConnection+0x36 libsldap.so.1`get_current_session+0x4d libsldap.so.1`search_state_machine+0x279 libsldap.so.1`ldap_list+0x378 libsldap.so.1`__ns_ldap_list_sort+0x8a ldaplist`list+0xbc ldaplist`main+0xa48 ldaplist`_start_crt+0x97 ldaplist`_start+0x1a input: "ecc423aad0" 1 66926 ascii2hex:return lenp 803fe7c len 5 output: 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef 0: ec c4 23 aa d0 ..#.. 1 66927 dvalue:return dvalue output(): "proxy"
The hex array returned by the function was the same before and after the change was applied, and the derived password string is indeed correct.
I also created a simple /etc/netgroup
file with the following contents:
admins (,root,) onemachine (onemachine,,) host_w_users (host,a,) (host,b,) everything (,,test)
Note that the local netgroup
file is not used directly by the system, but rather as input to the following test:
# for x in netgroup passwd group services auto_master; do ldapaddent -D cn=root -w secret -f /etc/$x $x ; done 4 entries added 27 entries added 29 entries added 205 entries added parse error: (line 29) 2 entries added
This test adds a number of directory entries using the local files for passwd
, group
, services
, auto_master
, and netgroup
. This represents most of the substantive changes to functions in ns_writes.c
. The parse error is the +auto_master
line in the default shipped file, which is an include directive rather than a real auto_master
entry.
I ran the ldapaddent
test before applying the change, then reset the directory contents to its original state and ran it again after applying the change. Though many directory entries were added:
# ldapsearch -h ldaptesting.example.com -D cn=root -w secret -b o=test dn | grep '^dn:' | head -20 dn: o=test dn: ou=users,o=test dn: ou=groups,o=test dn: ou=netgroups,o=test dn: ou=services,o=test dn: ou=proxies,o=test dn: cn=proxy,ou=proxies,o=test dn: ou=profile,o=test dn: cn=default,ou=profile,o=test dn: cn=admins,ou=netgroups,o=test dn: cn=onemachine,ou=netgroups,o=test dn: cn=host_w_users,ou=netgroups,o=test dn: cn=everything,ou=netgroups,o=test dn: uid=root,ou=users,o=test dn: uid=daemon,ou=users,o=test dn: uid=bin,ou=users,o=test dn: uid=sys,ou=users,o=test dn: uid=adm,ou=users,o=test dn: uid=lp,ou=users,o=test dn: uid=uucp,ou=users,o=test ... # ldapsearch -h ldaptesting.example.com -D cn=root -w secret -b o=test dn | grep '^dn:' | wc -l 277
... the only difference was in the salted/hashed password for the cn=proxy
bind account:
# diff -u before.ldif after.ldif --- before.ldif 2018-09-24 17:41:18.821063228 +0000 +++ after.ldif 2018-09-24 17:31:41.733538126 +0000 @@ -43,7 +43,7 @@ objectClass: applicationProcess objectClass: simpleSecurityObject cn: proxy -userPassword:: e1NTSEF9VzAzcmE4SkNmRkZYL0J5czhlYVQ5U3ZpdnViNGZ4MEI= +userPassword:: e1NTSEF9RlVnMjlTMWZ0NDhMWDVBWnpnbG4xbG5EUWlXeTZzRXo= # profile, test dn: ou=profile,o=test
Updated by Electric Monk almost 4 years ago
- Status changed from In Progress to Closed
- % Done changed from 90 to 100
git commit 91b658d374482612d0b9b29aed694a252c07286f
commit 91b658d374482612d0b9b29aed694a252c07286f Author: Toomas Soome <tsoome@me.com> Date: 2018-09-24T17:51:18.000Z 9169 libsldap: comparison between pointer and zero character constant Reviewed by: Yuri Pankov <yuripv@yuripv.net> Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk> Reviewed by: Ken Mays <kmays2000@gmail.com> Approved by: Joshua M. Clulow <josh@sysmgr.org>