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,