Hello bind developers!

I wanted to report a bug we found in sdbldap.c recently. I know sdb is a
contrib module so I am not sure if it is officially supported by the
bind-bugs maintainers the original author.

In short, several bugs could potentially be fixed by correcting a few
"ism's" related to potentially unsafe use of "free" in
contrib/sdb/ldap/ldapdb.c by adding appropriate if ( ... != NULL) checks
before things like free(conndata->index) at line 136... which is the
location that caused our particular crash, and free(threaddata->index) at
line 133, etc.

For best cross-platform/cross-compiler compatibility it would probably also
be good practice to be setting conndata->index = NULL after calling
free(conndata->index) so the if ( ... != NULL) checks would always prevent
double-freeing of blocks if the implementation of "free" doesn't happen to
also zero out the pointer on all platforms.

I am referring to what I hope is the latest code here:

https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD

The crash we encountered specifically occurred at line 136 below when
free(conndata->index) tried to free an invalid pointer.  Unfortunately the
code optimizer had optimized away conndata->index so I could not tell if it
was NULL or just a double-free of a dangling pointer reference.  But as
soon as I looked at the code and saw that they were checking for NULL
sometime and not other times, that seemed like a likely cause & good place
for a NULL check.

127
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l127>
        if (data == NULL) {
128
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l128>
                /* cleanup */
129
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l129>
                /* lock out other threads */
130
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l130>
                ldapdb_lock(1);
131
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l131>
                while (allthreadsdata != NULL) {
132
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l132>
                        threaddata = allthreadsdata;
133
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l133>
                        free(threaddata->index);
134
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l134>
                        while (threaddata->data != NULL) {
135
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l135>
                                conndata = threaddata->data;
136
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l136>
                               * free(conndata->index);*
137
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l137>
                                if (conndata->data != NULL)
138
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l138>
                                        ldap_unbind((LDAP *)conndata->data);
139
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l139>
                                threaddata->data = conndata->next;
140
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l140>
                                free(conndata);
141
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l141>
                        }
142
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l142>
                        allthreadsdata = threaddata->next;
143
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l143>
                        free(threaddata);
144
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l144>
                }
145
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l145>
                ldapdb_lock(-1);
146
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l146>
                return (NULL);
147
<https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=contrib/sdb/ldap/ldapdb.c;h=caade37c80e0a7ae8320d6faa3e921e094a90022;hb=HEAD#l147>
        }

I could supply a coredump file, but would probably be difficult to debug
unless you have an RHEL 6.5 machine with latest patches to debug with, so I
thought it would be better just to describe what appears to be a somewhat
pervasive problem in several places in this particular module.

If you would rather I compile up the latest code & try to make the fixes
myself, and then submit a patch against the latest code repo (assuming I am
looking at the latest repo) let me know and I would be glad to contribute.

Thanks for your support & contributions to the internet community.  Your
efforts are appreciated!

Troy