Ticket 571 - server does not accept 0 length LDAP Control sequence

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 length is not 0.

https://fedorahosted.org/389/ticket/571

Reviewed by richm(Thanks!)
(cherry picked from commit 36425a3d5875a05ee812c293b50eb0da44d8d39e)

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()  */
 

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) 	*/

ldap/servers/plugins/deref/deref.c

@@ -380,7 +380,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;

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;
     }
 

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;

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;

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;
 	}
@@ -1413,6 +1413,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;

ldap/servers/plugins/replication/windows_private.c

@@ -793,6 +793,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)

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 )) {

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;

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

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;
 	}
 

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.