Viewing Incoming/6785
Full headers

Major security issue: yes  no

Notes:

Notification:

Date: Wed, 12 Jan 2011 11:52:09 +0000
From: h.b.furuseth@usit.uio.no
To: openldap-its@OpenLDAP.org
Subject: slapo-chain mishandles sr_text/sr_matched

Full_Name: Hallvard B Furuseth
Version: HEAD
OS: 
URL: 
Submission from: (NULL) (193.157.198.89)
Submitted by: hallvard


ldap_chain_response() can send sr_text/sr_matched which refers to a
different error than sr_err and rc.

It saves rs-> sr_err, sr_text, sr_matched to three local variables and
can restore them later, but not maintain the text and matched variables
in parallel with err.  Also it does not track/reset/obey
REP_MATCHED_MUSTBEFREED along with sr_matched.

Fails test032-chain with the asserts below.  However there may be
non-success result with the wrong text/matched too, that cannot be
assert()ed.

Come to think of it, maybe the last issues applies to ITS#6774 too?
REP_MATCHED_MUSTBEFREED, mismatch between failure code and text/matched.

Index: servers/slapd/back-ldap/chain.c
@@ -1019,4 +1019,14 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 	rs->sr_ref = NULL;
 
+	const char *bad_incoming_matched = NULL, *bad_incoming_text = NULL;
+	switch ( sr_err ) {
+	case LDAP_SUCCESS:
+	case LDAP_COMPARE_TRUE:
+	case LDAP_COMPARE_FALSE:
+		bad_incoming_matched = matched;
+	case LDAP_REFERRAL:
+		bad_incoming_text = text;
+	}
+
 	/* we need this to know if back-ldap returned any result */
 	lb.lb_lc = lc;
@@ -1169,4 +1179,15 @@ cannot_chain:;
 
 dont_chain:;
+	switch ( sr_err ) {
+	case LDAP_SUCCESS:
+	case LDAP_COMPARE_TRUE:
+	case LDAP_COMPARE_FALSE:
+		assert( !matched || bad_incoming_matched );
+	case LDAP_REFERRAL:
+		assert( !text || bad_incoming_text );
+	}
+	assert( rc != LDAP_SUCCESS ||
+			(( !text    || bad_incoming_text) &&
+			 ( !matched || bad_incoming_matched )));
 	rs->sr_err = sr_err;
 	rs->sr_type = sr_type;

Followup 1

Date: Tue, 18 Jan 2011 23:31:46 +0100 (CET)
Subject: Re: (ITS#6785) slapo-chain mishandles sr_text/sr_matched
From: masarati@aero.polimi.it
To: h.b.furuseth@usit.uio.no
Cc: openldap-its@openldap.org

> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS:
> URL:
> Submission from: (NULL) (193.157.198.89)
> Submitted by: hallvard
>
>
> ldap_chain_response() can send sr_text/sr_matched which refers to a
> different error than sr_err and rc.
>
> It saves rs-> sr_err, sr_text, sr_matched to three local variables and
> can restore them later, but not maintain the text and matched variables
> in parallel with err.  Also it does not track/reset/obey
> REP_MATCHED_MUSTBEFREED along with sr_matched.
>
> Fails test032-chain with the asserts below.  However there may be
> non-success result with the wrong text/matched too, that cannot be
> assert()ed.
>
> Come to think of it, maybe the last issues applies to ITS#6774 too?
> REP_MATCHED_MUSTBEFREED, mismatch between failure code and text/matched.
>
> Index: servers/slapd/back-ldap/chain.c
> @@ -1019,4 +1019,14 @@ ldap_chain_response( Operation *op, SlapReply *rs )
>  	rs->sr_ref = NULL;
>
> +	const char *bad_incoming_matched = NULL, *bad_incoming_text = NULL;
> +	switch ( sr_err ) {
> +	case LDAP_SUCCESS:
> +	case LDAP_COMPARE_TRUE:
> +	case LDAP_COMPARE_FALSE:
> +		bad_incoming_matched = matched;
> +	case LDAP_REFERRAL:
> +		bad_incoming_text = text;
> +	}
> +
>  	/* we need this to know if back-ldap returned any result */
>  	lb.lb_lc = lc;
> @@ -1169,4 +1179,15 @@ cannot_chain:;
>
>  dont_chain:;
> +	switch ( sr_err ) {
> +	case LDAP_SUCCESS:
> +	case LDAP_COMPARE_TRUE:
> +	case LDAP_COMPARE_FALSE:
> +		assert( !matched || bad_incoming_matched );
> +	case LDAP_REFERRAL:
> +		assert( !text || bad_incoming_text );
> +	}
> +	assert( rc != LDAP_SUCCESS ||
> +			(( !text    || bad_incoming_text) &&
> +			 ( !matched || bad_incoming_matched )));
>  	rs->sr_err = sr_err;
>  	rs->sr_type = sr_type;

What do you suggest?  To use a local SlapReply instead of hijacking the
one passed as argument?

p.

Followup 2

From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 19 Jan 2011 09:25:43 +0100
To: masarati@aero.polimi.it
Cc: openldap-its@openldap.org
Subject: Re: (ITS#6785) slapo-chain mishandles sr_text/sr_matched

masarati@aero.polimi.it writes:
> What do you suggest?  To use a local SlapReply instead of hijacking the
> one passed as argument?

Not my call, I haven't looked enough at the code to know what it's doing.

-- 
Hallvard