Re: (ITS#4648) ldap_pvt_put_filter() succeeds on certain incorrect ldap filters

[Kurt@OpenLDAP.org]

At 02:37 PM 8/23/2006, john.bowers@quest.com wrote:
>Full_Name: John Bowers
>Version: 2.3.27
>OS: Suse Linux 9.1
>URL: ftp://ftp.openldap.org/incoming/john-bowers-06-08-23.patch

The patch is not yet available at this URL.  Though a copy is
provided in the body of the report, such copies cannot be used
with common developer tools (like patch) as they are generally
mangled.

I noticed that you included a "notice of origin" but did not
include a "rights statement".  Please provide both when
uploading the patch to the location specified above.

-- Kurt


>Submission from: (NULL) (192.41.88.6)
>
>
>The ldap_search code uses ldap_pvt_put_filter() to add the search filter to the
>search request.  Certain (if I am not enirely mistaken) invalid search filters
>are successfully added by the ldap_pvt_put_filter() code.  An example of this
>type of search filter follows:
>
>"(objectclass=group)(cn=test)"
>
>This sort of concatenation of search filters is always "successfully" processed
>by ldap_pvt_put_filter().  If ldap_pvt_put_filter() failed (as it easily could)
>then a more accurate error message would be delivered, and a error could be
>determined without causing network traffic.
>
>I have prepared a small patch, I have attempted to upload the patch to your ftp
>server, but it does not appear to be accepting working properly right now. 
>Because of this (and the fact that patch is rather small) I will include the
>patch text in this bug report.
>
>Essentially the patch just keeps track of the number of complex and simple
>filters that are added.  If there is more than 1 filter (any combination of
>complex or simple filters), ldap_pvt_put_filter() will fail appropriately.
>patch below
>------
>/* This patch file is derived from OpenLDAP Software.
> * All of the modifications to OpenLDAP Software represented
> * in the following patch(es) were developed by John Bowers
> * John.Bowers@quest.com. These modifications are not
> * subject to any license of Quest software. */
>--- openldap-2.3.27/libraries/libldap/filter.c  2006-01-03 15:16:08.000000000
>-0700
>+++ openldap-2.3.27_patch/libraries/libldap/filter.c    2006-08-23
>14:17:13.326437600 -0600
>@@ -331,7 +331,7 @@
>        char    *freeme;
>        char    *str;
>        char    *next;
>-       int     parens, balance, escape;
>+       int     parens, balance, escape, simple, complex;
>
>        /*
>         * A Filter looks like this:
>@@ -373,6 +373,8 @@
>        str = freeme;
>
>        parens = 0;
>+       simple = 0;
>+       complex = 0;
>        while ( *str ) {
>                switch ( *str ) {
>                case '(': /*')'*/
>@@ -392,6 +394,8 @@
>                                if( str == NULL ) {
>                                        rc = -1;
>                                        goto done;
>+                               } else {
>+                                       complex++;
>                                }
>
>                                parens--;
>@@ -406,6 +410,8 @@
>                                if( str == NULL ) {
>                                        rc = -1;
>                                        goto done;
>+                               } else {
>+                                       complex++;
>                                }
>
>                                parens--;
>@@ -420,6 +426,8 @@
>                                if( str == NULL ) {
>                                        rc = -1;
>                                        goto done;
>+                               } else {
>+                                       complex++;
>                                }
>
>                                parens--;
>@@ -461,6 +469,8 @@
>                                if ( put_simple_filter( ber, str ) == -1 ) {
>                                        rc = -1;
>                                        goto done;
>+                               } else {
>+                                       simple++;
>                                }
>
>                                *next++ = /*'('*/ ')';
>@@ -493,13 +503,15 @@
>                        if ( put_simple_filter( ber, str ) == -1 ) {
>                                rc = -1;
>                                goto done;
>+                       } else {
>+                               simple++;
>                        }
>                        str = next;
>                        break;
>                }
>        }
>
>-       rc = parens ? -1 : 0;
>+       rc = ( parens || ( simple + complex > 1 ) ) ? -1 : 0;
>
> done:
>        LDAP_FREE( freeme );