[Date Prev][Date Next] [Chronological] [Thread] [Top]
Saturn v1.1 on openldap v2.4.4alpha

To: openldap-bugs@openldap.org
Subject: Saturn v1.1 on openldap v2.4.4alpha
From: "Domagoj Babic" <babic.domagoj@gmail.com>
Date: Wed, 22 Aug 2007 02:19:51 -0700
Content-disposition: inline
Hi,

I'd appreciate if anyone could provide feedback on this report from
another tool. I've filtered out all the warnings that are provably false.


BTW, playing with openldap for a couple of days, I found that the code
is of much lower quality than some other open source projects, like
ISC bind & ntp, openssh,... Static checkers also find more bugs.

Some things I spotted:
- code is very non-uniform, the same things are done in very different
ways
- in several places (10-20) pointers are checked with an assertion, but
after the pointer has already been dereferenced. What's the purpose
of assertions then?
- "fat interfaces"
- very long sequences of dereferences, like:
*desc->ad_type->sat_equality->smr_normalize
make GDB debugging harder, discourage consistency checking,
and make static checking reports less readable.
- inconsistency, a lot of it, for instance, a pointer is dereferenced, and
ten lines below it's actually checked for NULL and then dereferenced


Anyways, here is the report, I hope it's going to be helpful:

Please replace ??? with Yes/No, and a short explanation. Bad programming
practices are not considered bugs. A bug (by my definition) is
a sequence of events that would crash the app, no matter how
small the probability is, or how irrelevant the app is. Please, also
consider the calling context (unlike Calysto, Saturn does not provide
a complete trace, so it is hard to figure out whether the functions
can be called under the right conditions to trigger the bug). If you
can prove that the function can never be called so as to trigger the
'bug', then it's not a bug.


----------------------------------------------------------------
Bug: ???

@blue
@2114 blue __arg0*.bv_val of function select_backend can evaluate to
NULL select_backend (2114:servers/slapd/acl.c),  final site of
dereference is: (678:servers/slapd/backend.c)
@servers/slapd/acl.c
@Null pointer is passed to a function which dereferences it.
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@violet
@774 violet (INCONSISTENT USE) Possible null dereference
__arg1*.e_nname.bv_val. This variable is checked for Null at lines:
704, and is dereferenced through call chain:
servers/slapd/acl.c:regex_matches (774:servers/slapd/acl.c),
acl_string_expand (2507:servers/slapd/acl.c),  final site of
dereference is: (2455:servers/slapd/acl.c)
@servers/slapd/acl.c
@Interprocedural inconsistency error

@violet
@837 violet (INCONSISTENT USE) Possible null dereference
__arg1*.e_nname.bv_val. This variable is checked for Null at lines:
704, and is dereferenced through call chain: acl_string_expand
(837:servers/slapd/acl.c),  final site of dereference is:
(2455:servers/slapd/acl.c)
@servers/slapd/acl.c
@Interprocedural inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like a bug.

@violet
@3868 violet (INCONSISTENT USE) Possible null dereference
__arg0*.si_anlist. This variable is checked for Null at lines: 3859,
and is dereferenced through call chain: anlist_unparse
(3868:servers/slapd/syncrepl.c),  final site of dereference is:
(2841:servers/slapd/bconfig.c)
@servers/slapd/syncrepl.c
@Interprocedural inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like a bug.

@red
@1753 red Possible NULL dereference of keys+keycount
@servers/slapd/schema_init.c
@Intraprocedural  Null error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@orange
@2212 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_suffix+i. This variable is checked for Null at lines: 2192
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2211 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_suffix+i. This variable is checked for Null at lines: 2192
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2212 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_suffix+(i+1). This variable is checked for Null at lines:
2192
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2209 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_suffix+i. This variable is checked for Null at lines: 2192
@servers/slapd/bconfig.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@orange
@3147 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_update_refs+i. This variable is checked for Null at lines:
3135
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@3149 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_update_refs+(i+1). This variable is checked for Null at
lines: 3135
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@3148 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_update_refs+i. This variable is checked for Null at lines:
3135
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@3149 orange (INCONSISTENT USE) Possible null dereference of variable
c->be->be_update_refs+i. This variable is checked for Null at lines:
3135
@servers/slapd/bconfig.c
@Inconsistency error
----------------------------------------------------------------


----------------------------------------------------------------
Bug: ???
There seems to be some correlation between default_passwd_hash and
c->valx interprocedurally, but I couldn't figure it out.

@orange
@1934 orange (INCONSISTENT USE) Possible null dereference of variable
default_passwd_hash+i. This variable is checked for Null at lines:
1938
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@1933 orange (INCONSISTENT USE) Possible null dereference of variable
default_passwd_hash+i. This variable is checked for Null at lines:
1938
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@1934 orange (INCONSISTENT USE) Possible null dereference of variable
default_passwd_hash+(i+1). This variable is checked for Null at lines:
1938
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@1932 orange (INCONSISTENT USE) Possible null dereference of variable
default_passwd_hash+i. This variable is checked for Null at lines:
1938
@servers/slapd/bconfig.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
The same as above

@orange
@2743 orange (INCONSISTENT USE) Possible null dereference of variable
default_referral+i. This variable is checked for Null at lines: 2731
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2744 orange (INCONSISTENT USE) Possible null dereference of variable
default_referral+i. This variable is checked for Null at lines: 2731
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2745 orange (INCONSISTENT USE) Possible null dereference of variable
default_referral+i. This variable is checked for Null at lines: 2731
@servers/slapd/bconfig.c
@Inconsistency error

@orange
@2745 orange (INCONSISTENT USE) Possible null dereference of variable
default_referral+(i+1). This variable is checked for Null at lines:
2731
@servers/slapd/bconfig.c
@Inconsistency error
----------------------------------------------------------------


----------------------------------------------------------------
Bug: ???

@orange
@377 orange (INCONSISTENT USE) Possible null dereference of variable
b1. This variable is checked for Null at lines: 348
@servers/slapd/backglue.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like a false positive, but not sure.

@violet
@463 violet (INCONSISTENT USE) Possible null dereference
servers/slapd/oc.c:oc_list.stqh_first*.soc_next.stqe_next. This
variable is checked for Null at lines: 461, and is dereferenced
through call chain: servers/slapd/oc.c:oc_delete_names
(463:servers/slapd/oc.c),  final site of dereference is:
(394:servers/slapd/oc.c)
@servers/slapd/oc.c
@Interprocedural inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@orange
@2807 orange (INCONSISTENT USE) Possible null dereference of variable
a->acl_attrs+0. This variable is checked for Null at lines: 2784
@servers/slapd/aclparse.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like attrs_alloc can return NULL @799.

@red
@869 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation

@red
@837 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation

@red
@871 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation

@red
@856 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation

@red
@840 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation

@red
@838 red Possible NULL dereference of a with trace a
@servers/slapd/entry.c
@Intraprocedural error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like out is initialized in the loop body, and the loop is executed
at least once, but I'm not sure.

@orange
@292 orange (INCONSISTENT USE) Possible null dereference of variable
out+outpos. This variable is checked for Null at lines: 268
@libraries/liblunicode/ucstr.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Bug, pointer returned by realloc not checked.

@red
@288 red Possible NULL dereference of values+(nvalues+j) with trace values
@tests/progs/slapd-search.c
@Intraprocedural error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like anew->an_name.bv_val can remain uninitialized or NULL

@red
@943 red Possible NULL dereference of anew->an_name.bv_val+0 with
trace anew*.an_name.bv_val
@servers/slapd/ad.c
@Intraprocedural error with loop summary propagation

@red
@921 red Possible NULL dereference of anew->an_name.bv_val+0 with
trace anew*.an_name.bv_val
@servers/slapd/ad.c
@Intraprocedural error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Inconsistency, can it be NULL?

@orange
@128 orange (INCONSISTENT USE) Possible null dereference of variable
info. This variable is checked for Null at lines: 136
@libraries/librewrite/rule.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@blue
@172 blue Trace entry of function tests/progs/slapd-modrdn.c:do_modrdn
evaluates to NULL and is dereferenced through call chain:
tests/progs/slapd-modrdn.c:do_modrdn (172:tests/progs/slapd-modrdn.c),
 final site of dereference is: (205:tests/progs/slapd-modrdn.c)
@tests/progs/slapd-modrdn.c
@Interprocedural Null error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like bugs - SF_POP can return NULL.

@blue
@385 blue __arg1 of function slap_set_join can evaluate to NULL
slap_set_join (385:servers/slapd/sets.c),  final site of dereference
is: (230:servers/slapd/sets.c)
@servers/slapd/sets.c
@Null pointer is passed to a function which dereferences it.

@blue
@411 blue __arg1 of function slap_set_join can evaluate to NULL
slap_set_join (411:servers/slapd/sets.c),  final site of dereference
is: (230:servers/slapd/sets.c)
@servers/slapd/sets.c
@Null pointer is passed to a function which dereferences it.
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like bugs.

@orange
@231 orange (INCONSISTENT USE) Possible null dereference of variable
rset+j. This variable is checked for Null at lines: 134, 188
@servers/slapd/sets.c
@Inconsistency error

@orange
@244 orange (INCONSISTENT USE) Possible null dereference of variable
rset+j. This variable is checked for Null at lines: 134, 188
@servers/slapd/sets.c
@Inconsistency error

@orange
@235 orange (INCONSISTENT USE) Possible null dereference of variable
rset+j. This variable is checked for Null at lines: 134, 188
@servers/slapd/sets.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like bugs.

@orange
@243 orange (INCONSISTENT USE) Possible null dereference of variable
lset+i. This variable is checked for Null at lines: 120, 188
@servers/slapd/sets.c
@Inconsistency error

@orange
@235 orange (INCONSISTENT USE) Possible null dereference of variable
lset+i. This variable is checked for Null at lines: 120, 188
@servers/slapd/sets.c
@Inconsistency error

@orange
@230 orange (INCONSISTENT USE) Possible null dereference of variable
lset+i. This variable is checked for Null at lines: 120, 188
@servers/slapd/sets.c
@Inconsistency error

@orange
@244 orange (INCONSISTENT USE) Possible null dereference of variable
lset+i. This variable is checked for Null at lines: 120, 188
@servers/slapd/sets.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
This pointer is checked @210, and if it's NULL a proper msg is printed,
and then the pointer can be dereferenced. Would this be considered a
bug, or a normal sequence of events?

@blue
@227 blue __arg0 of function servers/slurpd/ldap_op.c:free_ldmarr can
evaluate to NULL servers/slurpd/ldap_op.c:free_ldmarr
(227:servers/slurpd/ldap_op.c),  final site of dereference is:
(574:servers/slurpd/ldap_op.c)
@servers/slurpd/ldap_op.c
@Null pointer is passed to a function which dereferences it.
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Looks like a bug.

@red
@349 red Possible NULL dereference of ldmarr+nops
@servers/slurpd/ldap_op.c
@Intraprocedural  Null error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???

@red
@646 red Possible NULL dereference of srs with trace srs
@servers/slapd/overlays/syncprov.c
@Intraprocedural error with loop summary propagation

@red
@649 red Possible NULL dereference of srs with trace srs
@servers/slapd/overlays/syncprov.c
@Intraprocedural error with loop summary propagation

@red
@647 red Possible NULL dereference of srs with trace srs
@servers/slapd/overlays/syncprov.c
@Intraprocedural error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Can that pointer be NULL?

@orange
@545 orange (INCONSISTENT USE) Possible null dereference of variable
op->o_bd. This variable is checked for Null at lines: 551, 553, 597
@servers/slapd/backover.c
@Inconsistency error

@orange
@477 orange (INCONSISTENT USE) Possible null dereference of variable
op->o_bd. This variable is checked for Null at lines: 483, 485, 529
@servers/slapd/backover.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Looks like a bug if the loop body @1104 is not executed.

@red
@1112 red Possible NULL dereference of on with trace on
@servers/slapd/backover.c
@Intraprocedural error with loop summary propagation

@red
@1113 red Possible NULL dereference of on with trace on
@servers/slapd/backover.c
@Intraprocedural error with loop summary propagation
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Bad programming practice, what's the point of checking the pointer after
dereference?? Is this a bug?

@orange
@528 orange (INCONSISTENT USE) Possible null dereference of variable
val. This variable is checked for Null at lines: 534
@servers/slapd/value.c
@Inconsistency error

@orange
@465 orange (INCONSISTENT USE) Possible null dereference of variable
val. This variable is checked for Null at lines: 471
@servers/slapd/value.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Bad programming practice, is it a bug?

@orange
@298 orange (INCONSISTENT USE) Possible null dereference of variable
data. This variable is checked for Null at lines: 302
@libraries/librewrite/ldapmap.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like a bug.

@orange
@194 orange (INCONSISTENT USE) Possible null dereference of variable
defaults. This variable is checked for Null at lines: 115, 118, 121,
125
@libraries/liblutil/sasl.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
@red
@147 red Possible NULL dereference of p
@libraries/liblutil/avl.c
@Intraprocedural  Null error

@red
@148 red Possible NULL dereference of p
@libraries/liblutil/avl.c
@Intraprocedural  Null error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
This is definitely a bad programming practice. Can rdn be NULL?

@orange
@238 orange (INCONSISTENT USE) Possible null dereference of variable
rdn+iAVA. This variable is checked for Null at lines: 240
@servers/slapd/dn.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
This is definitely a bad programming practice. Can dn be NULL?

@orange
@1202 orange (INCONSISTENT USE) Possible null dereference of variable
dn. This variable is checked for Null at lines: 1204
@servers/slapd/dn.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
This is definitely a bad programming practice. Can suffix be NULL?

@orange
@1202 orange (INCONSISTENT USE) Possible null dereference of variable
suffix. This variable is checked for Null at lines: 1205
@servers/slapd/dn.c
@Inconsistency error
----------------------------------------------------------------

----------------------------------------------------------------
Bug: ???
Seems like a bug.

@red
@634 red Possible NULL dereference of avl_list+ciltmp
@libraries/liblutil/avl.c
@Intraprocedural  Null error
----------------------------------------------------------------





Thx,

-- 
        Domagoj Babic

        http://www.domagoj.info/
        http://www.calysto.org/
Follow-Ups:
- Re: Saturn v1.1 on openldap v2.4.4alpha
  - From: Kurt Zeilenga <kurt@OpenLDAP.org>
Prev by Date: (ITS#5101) Fatal Error: call to undefined function ldap_connect()
Next by Date: Re: (ITS#5024) Disabling pagedResults advertisements
Index(es):
- Chronological
- Thread