Fix GH-21262: ldap_modify() too strict controls argument validation.

make it impossible to unset an attribute.

close GH-21263

Author: devnexen

NEWS

@@ -33,6 +33,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-21097 (Accessing Dom\Node properties can can throw TypeError).
     (ndossche)
 
+- LDAP:
+  . Fixed bug GH-21262 (ldap_modify() too strict controls argument validation
+    makes it impossible to unset attribute). (David Carlier)
+
 - MBString:
   . Fixed bug GH-21223; mb_guess_encoding no longer crashes when passed huge
     list of candidate encodings (with 200,000+ entries). (Jordi Kroon)

ext/ldap/ldap.c

@@ -2339,9 +2339,14 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, bool ext)
 			SEPARATE_ARRAY(attribute_values);
 			uint32_t num_values = zend_hash_num_elements(Z_ARRVAL_P(attribute_values));
 			if (num_values == 0) {
-				zend_argument_value_error(3, "attribute \"%s\" must be a non-empty list of attribute values", ZSTR_VAL(attribute));
-				RETVAL_FALSE;
-				goto cleanup;
+				if (UNEXPECTED(oper == LDAP_MOD_ADD)) {
+					zend_argument_value_error(3, "attribute \"%s\" must be a non-empty list of attribute values", ZSTR_VAL(attribute));
+					RETVAL_FALSE;
+					goto cleanup;
+				}
+				/* When we modify, we mean we delete the attribute */
+				attribute_index++;
+				continue;
 			}
 			if (!php_ldap_is_numerically_indexed_array(Z_ARRVAL_P(attribute_values))) {
 				zend_argument_value_error(3, "attribute \"%s\" must be an array of attribute values with numeric keys", ZSTR_VAL(attribute));

ext/ldap/tests/gh21262.phpt

@@ -0,0 +1,50 @@
+--TEST--
+GH-21262 (ldap_modify() too strict controls argument validation)
+--EXTENSIONS--
+ldap
+--FILE--
+<?php
+/* We are assuming 3333 is not connectable */
+$ldap = ldap_connect('ldap://127.0.0.1:3333');
+$valid_dn = "cn=userA,something";
+
+$entry_with_empty_array = [
+	'attribute1' => 'value',
+	'attribute2' => [],
+];
+
+// ldap_add() should still reject empty arrays
+try {
+	ldap_add($ldap, $valid_dn, $entry_with_empty_array);
+} catch (ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+
+// ldap_mod_add() should still reject empty arrays
+try {
+	ldap_mod_add($ldap, $valid_dn, $entry_with_empty_array);
+} catch (ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+
+// ldap_modify() should accept empty arrays (delete attribute)
+try {
+	@ldap_modify($ldap, $valid_dn, $entry_with_empty_array);
+	echo "ldap_modify: no ValueError thrown", PHP_EOL;
+} catch (ValueError $e) {
+	echo "ldap_modify: UNEXPECTED ValueError: ", $e->getMessage(), PHP_EOL;
+}
+
+// ldap_mod_del() should accept empty arrays (delete attribute)
+try {
+	@ldap_mod_del($ldap, $valid_dn, $entry_with_empty_array);
+	echo "ldap_mod_del: no ValueError thrown", PHP_EOL;
+} catch (ValueError $e) {
+	echo "ldap_mod_del: UNEXPECTED ValueError: ", $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECT--
+ldap_add(): Argument #3 ($entry) attribute "attribute2" must be a non-empty list of attribute values
+ldap_mod_add(): Argument #3 ($entry) attribute "attribute2" must be a non-empty list of attribute values
+ldap_modify: no ValueError thrown
+ldap_mod_del: no ValueError thrown