(ITS#4405) authentication crash in recursive referrals

[wenwu.lu@gmail.com]

Full_Name: Bin Lu
Version: 2.1.22/2.3.19
OS: Linux
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (66.129.224.36)


Note the problem reported below mainly happens on 2.1.22. But did verify it
happens on 2.3.19 as well. All the debuggings and our temporary fix were done
with 2.1.22. 
Also note, this problem only happens on our customer's environment from where we
are
not able to obtain the test data. We also regret that ldapsearch command
provided by
openldap does not support non-anonymous referral chasing, so no output of this
command is provided which might have been helpful.

We are using openldap in our product for our authentication module to
authenticate against Ldap/AD(not running openldap). So the code does not involve
openldap slapd.

In an Ldap/AD environment with heavy referral settings, authentication
occasionally fails due to crash in openldap code, see below stack trace:

Source                                        Function
------                                        --------
??:0                                          ??
??:0                                          ldap_result
??:0                                          ldap_sasl_bind
??:0                                          ldap_sasl_interactive_bind_s
install/include/dsutil/str.h:595              DSLdapServer::connect(DSStatus *)
install/include/dsutil/str.h:595              DSLdapServer::connect(DSStatus *)
??:0                                          ldap_new_connection
??:0                                          ldap_send_server_request
??:0                                          ldap_chase_v3referrals
??:0                                          try_read1msg
??:0                                          wait4msg
??:0                                          ldap_result

It shows the code entered referral chasing, and then rebind proc provided by
us, and then ldap_sasl_bind. Our debugging shows the code actually crashes in
libraries/libldap/result.c at(2.1.22 code):

            while (rc == -2) {
                    :
                    :
                 for ( lc = ld->ld_conns; rc == -2 && lc != NULL;
                      lc = nextlc ) {
                      nextlc = lc->lconn_next;
                      if ( lc->lconn_status ==
                           LDAP_CONNST_CONNECTED &&
                           ldap_is_read_ready( ld,
                           lc->lconn_sb )) {
                           rc = try_read1msg(ld, msgid, all,
                                lc->lconn_sb,lc, result );
                      }
                }
                     :
            }

In the for loop, the 'nextlc' in some cases is not NULL but invalid.
It either causes an assertion in ldap_is_read_ready() or SEGV in access 
to its pointer. Further debugging shows in the case of assertion failure 
or SEGV crash, the address of 'nextlc' (assigned to 'lc' the next round) 
is freed in ldap_free_connection() before it reaches this loop. 

Saw some code changes in 2.3.19 release(see below) in this area but 
obviously it doesn't fix this problem. 

             rc = try_read1msg( ld, msgid, all,
                  lc->lconn_sb, &lc, result );
             if ( lc == NULL ) lc = nextlc;

The LdapConn parameter becomes a pointer of pointer and it could be 
reset to NULL, which implies the try_read1msg() call can cause the 
link list node to be freed. But this if statement does not do any
help.

To verify our findings, we made the following change. If the problem
is caused by the fact that 'nextlc'pointer is freed after try_read1msg() 
call, then starting over from the head should solve the problem.

         for ( lc = ld->ld_conns; rc == -2 && lc != NULL;
             lc = nextlc ) {

             tmplc = ld->ld_conns;
             foundinlist = 0; 
             while (tmplc != NULL) {
                 if (tmplc != lc)
                     tmplc = tmplc->lconn_next;
                 else {
                     foundinlist = 1;
                     break; 
                 }
             }
             if (foundinlist == 0) {
                 // lc is freed already. start over. 
                 nextlc = ld->ld_conns;
             } else {
                 nextlc = lc->lconn_next;
                 if ( lc->lconn_status ==
                      LDAP_CONNST_CONNECTED &&
                      ldap_is_read_ready( ld,
                      lc->lconn_sb )) {
                      rc = try_read1msg( ld, msgid, all,
                           lc->lconn_sb, lc, result );
                 }
              }
           }

After this change, the above authentication failure disappeared.

This may not look the best solution. I believe that you can provide
better codes with your much deeper understanding of the code in general
to prevent accessing to the LdapConn object being freed by try_read1msg()
in this for loop.

Regards and thanks,
-binlu