ext/ldap: Fix GH-16132 (Freeing pointer not allocated by ZMM)

[Girgias]

No description provided.

[nielsdos]

Fix for both memleak and crash seem good. I have one suggestion though to avoid code duplication.

[nielsdos]

I would replace the entire "Free allocated memory" block with just:

			RETVAL_FALSE;
			num_berval[i] = 0;
			num_attribs = i + 1;
			ldap_mods[i]->mod_bvalues = NULL;
			goto cleanup;

Sadly I can't mark it as a suggestion

[Girgias]

I tried doing that but I think my issues was not setting mod_bvalues to NULL

[nielsdos]

This works for me:

diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c
index d40bf550260..82d4975a769 100644
--- a/ext/ldap/ldap.c
+++ b/ext/ldap/ldap.c
@@ -2236,22 +2236,11 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext)
 			ldap_mods[i]->mod_type = estrndup(ZSTR_VAL(attribute), ZSTR_LEN(attribute));
 		} else {
 			php_error_docref(NULL, E_WARNING, "Unknown attribute in the data");
-			/* Free allocated memory */
-			while (i >= 0) {
-				if (ldap_mods[i]->mod_type) {
-					efree(ldap_mods[i]->mod_type);
-					/* Free attribute values */
-					for (j = 0; j < num_berval[i]; j++) {
-						efree(ldap_mods[i]->mod_bvalues[j]);
-					}
-					efree(ldap_mods[i]->mod_bvalues);
-				}
-				efree(ldap_mods[i]);
-				i--;
-			}
-			efree(num_berval);
-			efree(ldap_mods);
-			RETURN_FALSE;
+			RETVAL_FALSE;
+			num_berval[i] = 0;
+			num_attribs = i + 1;
+			ldap_mods[i]->mod_bvalues = NULL;
+			goto cleanup;
 		}
 
 		value = zend_hash_get_current_data(Z_ARRVAL_P(entry));

[nielsdos]

Looks fine
ASAN job failure is because of the JSON stack limit test I added, which works for some reason on 8.3 but not on higher versions...