Re: (ITS#8385) Use After Free of struct ldap_common in slap_client_connect

[hyc@symas.com]

mkp37215@gmail.com wrote:
> Full_Name: Maciej Puzio
> Version: 2.4.44 and current sources from git repo
> OS: Ubuntu 15.10 amd64
> URL:
> Submission from: (NULL) (129.112.109.41)
>
>
> While configuring two servers in a dual-master setup I encountered an issue
> causing replication failures, when replication is done on a TLS connection. In
> this particular setup the issue manifested itself by replication retries
> failing, even when the cause of the original failure no longer existed. For
> example, let's consider that dual-master setup consists of servers A and B. If
> server A was started before server B, its on-startup replication would fail (as
> expected, since B is down). Server B would then be started and its replication
> attempt would succeed (since A is already running). However A's subsequent
> retries would fail, even though B was now accessible. If I then restarted server
> A, its replication will succeed, but B will now lose connection to A and all B's
> subsequent retries would fail. This issue has ~50% reproducibility, but if it
> occurs once, then it would continue failing. On occasion both servers would
> replicate fine, but obviously the whole situation was unacceptable on servers
> being prepared for production use.
>
> I tested this issue thoroughly eliminating various hypotheses, and ended up
> debugging OpenLDAP code and finding a bug causing the above behavior. In short,
> on failure slap_client_connect deallocates LDAP object ld and objects pointed by
> it, including ldap_common which contains ld_options. Pointer to ld_options is
> held in sb->sb_tls_ctx and reused in subsequent slap_client_connect calls, at
> this point referencing freed memory.
>
> This issue occurs in OpenLDAP newest stable version 2.4.44, as well as in
> current source code from git repo (as of March 11, 2016), obtained from
> git://git.openldap.org/openldap.git

> Here, instead of freeing ld->ldc (struct ldap_common), I overwrite its contents
> with a magic debug value 0xdeadbeef. I overwrite ldo_tls_require_cert, as an
> incorrect value of this particular variable turned out to be the direct cause of
> replication failures mentioned at the beginning (more about it later). However,
> other fields of ldap_common could be used as well.
>
> File ldap-int.h is included to be able to examine contents of *ld, while the
> declaration of struct tlsg_ctx has been copied verbatim from
> libraries/libldap/tls_g.c, in order to examine the contents of
> ld->ld_options.ldo_tls_ctx. I use GnuTLS library following Ubuntu build options.
> But the issue in question is not GnuTLS-related, though it may cause different
> symptoms with OpenSSL or Mozilla NSS (I did not test these options).

Thanks for the report. Inspection shows that the issue only exists in 
libldap's GnuTLS support. As always, the Project recommends you use OpenSSL.

It also looks like only the ldo_tls_require_cert field is being used so we can 
probably just copy that flag instead of keeping a pointer to the ldap options 
structure.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/