Attachment 701393 Details for Bug 912964 – revised git patch file (389-ds-base-1.2.11)

[patch] revised git patch file (389-ds-base-1.2.11)

0001-Bugzilla-912964-bug-in-handling-of-LDAPv3-control-da.patch (text/plain), 11.85 KB, created by Noriko Hosoi on 2013-02-22 22:44:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Noriko Hosoi

Created: 2013-02-22 22:44:39 UTC

Size: 11.85 KB

patch

obsolete

>From ec249b18a49b5036bf7c5a1530f686c3d27c5a19 Mon Sep 17 00:00:00 2001
>From: Noriko Hosoi <nhosoi@redhat.com>
>Date: Fri, 22 Feb 2013 14:40:46 -0800
>Subject: [PATCH] Bugzilla 912964 - bug in handling of LDAPv3 control data
>
>Bug Description:
>If we receive an incorrect control, with non NULL bv_val but
>bv_len=0, it can trigger a crash.  This bug occurs because
>of a call of openldap ber_init with such incorrect control.
>
>Fix Description:
>Before calling ber_init, check that the control lenght is not 0.
>
>Note: checking in the patch on behalf of "Thierry bordaz"
><tbordaz@redhat.com>
>
>https://bugzilla.redhat.com/show_bug.cgi?id=913228
>
>Reviewed by: rmeggins@redhat.com, nhosoi@redhat.com
>
>Platforms tested: Fedora 18, RHEL 6.4 (RHDS6.4)
>
>Flag Day: no
>
>Doc impact: possibly document a security bug
>---
> ldap/servers/plugins/chainingdb/cb_controls.c      |  6 ++----
> ldap/servers/plugins/chainingdb/cb_utils.c         |  2 +-
> ldap/servers/plugins/deref/deref.c                 |  2 +-
> ldap/servers/plugins/dna/dna.c                     |  7 ++++---
> ldap/servers/plugins/replication/repl5_total.c     |  6 +++---
> ldap/servers/plugins/replication/repl_controls.c   |  2 +-
> ldap/servers/plugins/replication/repl_extop.c      | 21 +++++++++++++--------
> ldap/servers/plugins/replication/windows_private.c |  7 +++++++
> ldap/servers/slapd/back-ldbm/sort.c                |  9 ++++-----
> ldap/servers/slapd/back-ldbm/vlv.c                 |  4 ++--
> ldap/servers/slapd/passwd_extop.c                  |  2 +-
> ldap/servers/slapd/proxyauth.c                     |  2 +-
> ldap/servers/slapd/slapi-plugin.h                  |  4 ++++
> 13 files changed, 44 insertions(+), 30 deletions(-)
>
>diff --git a/ldap/servers/plugins/chainingdb/cb_controls.c b/ldap/servers/plugins/chainingdb/cb_controls.c
>index f079901..2a698a0 100644
>--- a/ldap/servers/plugins/chainingdb/cb_controls.c
>+++ b/ldap/servers/plugins/chainingdb/cb_controls.c
>@@ -220,10 +220,8 @@ int cb_update_controls( Slapi_PBlock * pb,
>             ctrls[dCount]=slapi_dup_control(reqControls[cCount]);
>             dCount++;
> 
>-        } else
>-            if (!strcmp(reqControls[cCount]->ldctl_oid,CB_LDAP_CONTROL_CHAIN_SERVER) &&
>-                reqControls[cCount]->ldctl_value.bv_val) {
>-
>+        } else if (!strcmp(reqControls[cCount]->ldctl_oid,CB_LDAP_CONTROL_CHAIN_SERVER) &&
>+                   BV_HAS_DATA((&(reqControls[cCount]->ldctl_value)))) {
>             /* Max hop count reached ?                 */
>             /* Checked earlier by a call to cb_forward_operation()  */
> 
>diff --git a/ldap/servers/plugins/chainingdb/cb_utils.c b/ldap/servers/plugins/chainingdb/cb_utils.c
>index 2492226..f28f5d3 100644
>--- a/ldap/servers/plugins/chainingdb/cb_utils.c
>+++ b/ldap/servers/plugins/chainingdb/cb_utils.c
>@@ -148,7 +148,7 @@ int cb_forward_operation(Slapi_PBlock * pb ) {
> 		int iscritical=0;
> 
> 		if (slapi_control_present(ctrls,CB_LDAP_CONTROL_CHAIN_SERVER,&ctl_value,&iscritical) &&
>-			ctl_value && ctl_value->bv_val) {
>+			BV_HAS_DATA(ctl_value)) {
> 
> 			/* Decode control data 			*/
> 			/* hop           INTEGER (0 .. maxInt) 	*/
>diff --git a/ldap/servers/plugins/deref/deref.c b/ldap/servers/plugins/deref/deref.c
>index 7c502df..51394c8 100644
>--- a/ldap/servers/plugins/deref/deref.c
>+++ b/ldap/servers/plugins/deref/deref.c
>@@ -382,7 +382,7 @@ deref_parse_ctrl_value(DerefSpecList *speclist, const struct berval *ctrlbv, int
> 
>     PR_ASSERT(ctrlbv && ctrlbv->bv_val && ctrlbv->bv_len && ldapcode && ldaperrtext);
> 
>-    if (!ctrlbv || !ctrlbv->bv_val) {
>+    if (!BV_HAS_DATA(ctrlbv)) {
>         *ldapcode = LDAP_PROTOCOL_ERROR;
>         *ldaperrtext = "Empty deref control value";
>         return;
>diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c
>index 6babe23..080e357 100644
>--- a/ldap/servers/plugins/dna/dna.c
>+++ b/ldap/servers/plugins/dna/dna.c
>@@ -1668,7 +1668,7 @@ static int dna_request_range(struct configEntry *config_entry,
>     }
> 
>     /* Parse response */
>-    if (responsedata && responsedata->bv_val) {
>+    if (BV_HAS_DATA(responsedata)) {
>         respber = ber_init(responsedata);
>         if (ber_scanf(respber, "{aa}", &lower_str, &upper_str) == LBER_ERROR) {
>             ret = LDAP_PROTOCOL_ERROR;
>@@ -3745,14 +3745,15 @@ static int dna_extend_exop(Slapi_PBlock *pb)
> 
>     /* Fetch the request data */
>     slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &reqdata);
>-    if (!reqdata || !reqdata->bv_val) {
>+    if (!BV_HAS_DATA(reqdata)) {
>         slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
>                         "dna_extend_exop: No request data received.\n");
>         goto free_and_return;
>     }
> 
>     /* decode the exop */
>-    if ((reqdata->bv_val == NULL) || (tmp_bere = ber_init(reqdata)) == NULL) {
>+    tmp_bere = ber_init(reqdata);
>+    if (tmp_bere == NULL) {
>         goto free_and_return;
>     }
> 
>diff --git a/ldap/servers/plugins/replication/repl5_total.c b/ldap/servers/plugins/replication/repl5_total.c
>index 99ba838..f22246d 100644
>--- a/ldap/servers/plugins/replication/repl5_total.c
>+++ b/ldap/servers/plugins/replication/repl5_total.c
>@@ -726,10 +726,10 @@ decode_total_update_extop(Slapi_PBlock *pb, Slapi_Entry **ep)
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_OID, &extop_oid);
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
> 
>-	if (NULL == extop_oid ||
>+	if ((NULL == extop_oid) || 
> 		((strcmp(extop_oid, REPL_NSDS50_REPLICATION_ENTRY_REQUEST_OID) != 0) && 
>-		(strcmp(extop_oid, REPL_NSDS71_REPLICATION_ENTRY_REQUEST_OID) != 0)) ||
>-		NULL == extop_value || NULL == extop_value->bv_val)
>+		 (strcmp(extop_oid, REPL_NSDS71_REPLICATION_ENTRY_REQUEST_OID) != 0)) ||
>+		!BV_HAS_DATA(extop_value))
> 	{
> 		/* Bogus */
> 		goto loser;
>diff --git a/ldap/servers/plugins/replication/repl_controls.c b/ldap/servers/plugins/replication/repl_controls.c
>index 980bdd8..e6aeaaa 100644
>--- a/ldap/servers/plugins/replication/repl_controls.c
>+++ b/ldap/servers/plugins/replication/repl_controls.c
>@@ -216,7 +216,7 @@ decode_NSDS50ReplUpdateInfoControl(LDAPControl **controlsp,
> 	if (slapi_control_present(controlsp, REPL_NSDS50_UPDATE_INFO_CONTROL_OID,
> 	    &ctl_value, &iscritical))
> 	{
>-		if ((ctl_value->bv_val == NULL) || (tmp_bere = ber_init(ctl_value)) == NULL)
>+		if (!BV_HAS_DATA(ctl_value) || (tmp_bere = ber_init(ctl_value)) == NULL)
> 		{
> 			rc = -1;
> 			goto loser;
>diff --git a/ldap/servers/plugins/replication/repl_extop.c b/ldap/servers/plugins/replication/repl_extop.c
>index e842c62..68aed62 100644
>--- a/ldap/servers/plugins/replication/repl_extop.c
>+++ b/ldap/servers/plugins/replication/repl_extop.c
>@@ -343,10 +343,10 @@ decode_startrepl_extop(Slapi_PBlock *pb, char **protocol_oid, char **repl_root,
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_OID, &extop_oid);
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
> 
>-	if (NULL == extop_oid ||
>+	if ((NULL == extop_oid) ||
> 		((strcmp(extop_oid, REPL_START_NSDS50_REPLICATION_REQUEST_OID) != 0) &&
>-		(strcmp(extop_oid, REPL_START_NSDS90_REPLICATION_REQUEST_OID) != 0)) ||
>-		NULL == extop_value || NULL == extop_value->bv_val)
>+		 (strcmp(extop_oid, REPL_START_NSDS90_REPLICATION_REQUEST_OID) != 0)) ||
>+		!BV_HAS_DATA(extop_value))
> 	{
> 		/* bogus */
> 		rc = -1;
>@@ -480,9 +480,9 @@ decode_endrepl_extop(Slapi_PBlock *pb, char **repl_root)
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_OID, &extop_oid);
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
> 
>-	if (NULL == extop_oid ||
>-		strcmp(extop_oid, REPL_END_NSDS50_REPLICATION_REQUEST_OID) != 0 ||
>-		NULL == extop_value || NULL == extop_value->bv_val)
>+	if ((NULL == extop_oid) || 
>+	    (strcmp(extop_oid, REPL_END_NSDS50_REPLICATION_REQUEST_OID) != 0) ||
>+	    !BV_HAS_DATA(extop_value))
> 	{
> 		/* bogus */
> 		rc = -1;
>@@ -545,8 +545,8 @@ decode_repl_ext_response(struct berval *bvdata, int *response_code,
> 	PR_ASSERT(NULL != response_code);
> 	PR_ASSERT(NULL != ruv_bervals);
> 
>-	if (NULL == bvdata || NULL == response_code || NULL == ruv_bervals ||
>-		NULL == data_guid || NULL == data || NULL == bvdata->bv_val)
>+	if ((NULL == response_code) || (NULL == ruv_bervals) ||
>+	    (NULL == data_guid) || (NULL == data) || !BV_HAS_DATA(bvdata))
> 	{
> 		return_value = -1;
> 	}
>@@ -1412,6 +1412,11 @@ decode_cleanruv_payload(struct berval *extop_value, char **payload)
> {
> 	BerElement *tmp_bere = NULL;
> 	int rc = 0;
>+        
>+	if (!BV_HAS_DATA(extop_value)) {
>+		rc = -1;
>+		goto free_and_return;                    
>+	}
> 
> 	if ((tmp_bere = ber_init(extop_value)) == NULL){
> 		rc = -1;
>diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c
>index 355abd5..20360a3 100644
>--- a/ldap/servers/plugins/replication/windows_private.c
>+++ b/ldap/servers/plugins/replication/windows_private.c
>@@ -782,6 +782,13 @@ void windows_private_update_dirsync_control(const Repl_Agmt *ra,LDAPControl **co
> 			dirsync = slapi_dup_control( controls[i-1]);
> 		}
> 
>+		if (!dirsync || !BV_HAS_DATA((&(dirsync->ldctl_value)))) {
>+#ifdef FOR_DEBUGGING
>+			return_value = LDAP_CONTROL_NOT_FOUND;
>+#endif
>+			goto choke;
>+		}
>+                
> 		ber = ber_init( &dirsync->ldctl_value ) ;
> 
> 		if (ber_scanf( ber, "{iiO}", &hasMoreData, &maxAttributeCount, &serverCookie) == LBER_ERROR)
>diff --git a/ldap/servers/slapd/back-ldbm/sort.c b/ldap/servers/slapd/back-ldbm/sort.c
>index 501765c..6984467 100644
>--- a/ldap/servers/slapd/back-ldbm/sort.c
>+++ b/ldap/servers/slapd/back-ldbm/sort.c
>@@ -299,15 +299,14 @@ int parse_sort_spec(struct berval *sort_spec_ber, sort_spec **ps)
> 	char *matchrule = NULL;
> 	int rc = LDAP_SUCCESS;
> 
>-	if (NULL == sort_spec_ber->bv_val) {
>+	if (!BV_HAS_DATA(sort_spec_ber)) {
> 		return LDAP_PROTOCOL_ERROR;
> 	}
> 
> 	ber = ber_init(sort_spec_ber);
>-    if(ber==NULL)
>-    {
>-        return -1;
>-    }
>+	if (ber == NULL) {
>+		return -1;
>+	}
> 
> 	/* Work our way along the BER, one sort spec at a time */
> 	for ( tag = ber_first_element( ber, &len, &last ); !term_tag(tag); tag = ber_next_element( ber, &len, last )) {
>diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c
>index 39d16bf..2c09531 100644
>--- a/ldap/servers/slapd/back-ldbm/vlv.c
>+++ b/ldap/servers/slapd/back-ldbm/vlv.c
>@@ -1861,11 +1861,11 @@ vlv_parse_request_control( backend *be, struct berval *vlv_spec_ber,struct vlv_r
>    	*/
>     BerElement *ber = NULL;
>     int return_value = LDAP_SUCCESS;
>-	
>+
>     vlvp->value.bv_len = 0;
>     vlvp->value.bv_val = NULL;
> 
>-    if (NULL == vlv_spec_ber->bv_val)
>+    if (!BV_HAS_DATA(vlv_spec_ber))
>     {
>         return_value= LDAP_OPERATIONS_ERROR;
>         return return_value;
>diff --git a/ldap/servers/slapd/passwd_extop.c b/ldap/servers/slapd/passwd_extop.c
>index 3c050d6..4aebee3 100644
>--- a/ldap/servers/slapd/passwd_extop.c
>+++ b/ldap/servers/slapd/passwd_extop.c
>@@ -534,7 +534,7 @@ passwd_modify_extop( Slapi_PBlock *pb )
> 	/* Get the ber value of the extended operation */
> 	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
> 
>-	if (extop_value->bv_val == NULL)
>+	if (!BV_HAS_DATA(extop_value))
> 	{
> 		/* The request field wasn't provided.  We'll
> 		 * now try to determine the userid and verify
>diff --git a/ldap/servers/slapd/proxyauth.c b/ldap/servers/slapd/proxyauth.c
>index fe36cf1..562ac93 100644
>--- a/ldap/servers/slapd/proxyauth.c
>+++ b/ldap/servers/slapd/proxyauth.c
>@@ -96,7 +96,7 @@ parse_LDAPProxyAuth(struct berval *spec_ber, int version, char **errtextp,
> 		break;
> 	}
> 
>-	if ( !spec_ber || !spec_ber->bv_val ) {
>+	if (!BV_HAS_DATA(spec_ber)) {
> 		break;
> 	}
> 
>diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
>index 4962b2a..f78787e 100644
>--- a/ldap/servers/slapd/slapi-plugin.h
>+++ b/ldap/servers/slapd/slapi-plugin.h
>@@ -107,6 +107,10 @@ NSPR_API(PRUint32) PR_fprintf(struct PRFileDesc* fd, const char *fmt, ...)
> #define BERLEN_T "u"
> #endif
> 
>+/* Common check on berval before accessing the contents. */
>+/* bv is a struct berval *bv */
>+#define BV_HAS_DATA(bv) ((bv != NULL) && (bv->bv_len > 0) && (bv->bv_val != NULL))
>+
> /*
>  * The slapi_attr_get_flags() routine returns a bitmap that contains one or
>  * more of these values.
>-- 
>1.7.11.7
>

Flags:

rmeggins: review+

Actions: View | Diff

Attachments on bug 912964: 701393