(ITS#5154) Sequential binds to back-meta cause assertion failiure

[mhardin@symas.com]

Full_Name: Matthew Hardin
Version: 2.3.38
OS: N/A
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (12.168.121.2)


Multiple binds on the same connection where at least one bind ends up being
passed through to a target and where the credentials for that bind are incorrect
will cause back-meta to throw an assertion failure during a subsequent bind. In
this situation back-meta prints the message "slapd: bind.c:229: meta_back_bind:
Assertion `tmpmc == mc' failed".

Analysis:
In bind.c there is no cleanup of the metaconn structure after a failed bind to a
target database. This is ordinarily not a problem, as most applications perform
an unbind immediately after a failed bind and so fail to trigger this bug.
However, applications such as pam_ldap will perform several binds to as many as
two different identities on the same connection, and the credentials for one of
those identities are routinely incorrect. This bug causes the metaconn structure
from the failed bind to be left in the mi_conninfo.lai_tree avl tree and then
found during the re-insertion of a metaconn structure from a subsequent bind
(approx between lines 209-258 in bind.c), causing the assertion failure and
killing slapd in the process.

Note: This bug can be exploited as a DOS attack, as the behavior is relatively
easy to reproduce on an unpatched system with a short perl script.


Proposed Fix:
The simplest and best fix appears to be to mark the metaconn struct from the
failed bind as tainted. This causes the struct to be deleted in the call to
meta_back_release_conn around line 281. 

The patch is as follows:

bind.c
***************
*** 189,194 ****
--- 189,198 ----

                if ( lerr != LDAP_SUCCESS ) {
                        rc = rs->sr_err = lerr;
+                       /* Mark the meta_conn struct as tainted so
+                        * it'll be freed by meta_conn_back_destroy below */
+                       LDAP_BACK_CONN_TAINTED_SET( mc );
+
                        /* FIXME: in some cases (e.g. unavailable)
                         * do not assume it's not candidate; rather
                         * mark this as an error to be eventually