Project

General

Profile

Bug #9169

libsldap: comparison between pointer and zero character constant

Added by Toomas Soome over 1 year ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
2018-02-22
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

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

History

#1

Updated by Toomas Soome over 1 year ago

  • Status changed from New to In Progress
#2

Updated by Joshua M. Clulow 10 months 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
#3

Updated by Electric Monk 10 months 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>

Also available in: Atom PDF