Merge pull request #3191 from marcstern/v2/pr/mem_leak_re

Memory leaks + enhanced logging
This commit is contained in:
Ervin Hegedus
2024-08-26 16:37:01 +02:00
committed by GitHub
2 changed files with 95 additions and 142 deletions

View File

@@ -1475,7 +1475,8 @@ void sec_audit_logger_json(modsec_rec *msr) {
/* Unlock the mutex we used to serialise access to the audit log file. */ /* Unlock the mutex we used to serialise access to the audit log file. */
rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock); rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
get_apr_error(msr->mp, rc)); get_apr_error(msr->mp, rc));
} }
@@ -2256,7 +2257,8 @@ void sec_audit_logger_native(modsec_rec *msr) {
/* Unlock the mutex we used to serialise access to the audit log file. */ /* Unlock the mutex we used to serialise access to the audit log file. */
rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock); rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
get_apr_error(msr->mp, rc)); get_apr_error(msr->mp, rc));
} }

View File

@@ -254,24 +254,22 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
int i, rc, match = 0, var_appended = 0; int i, rc, match = 0, var_appended = 0;
if (rule != NULL) { if (rule != NULL) {
target_list = strdup(p2); target_list = strdup(p2);
if(target_list == NULL) if (target_list == NULL) {
return apr_psprintf(ruleset->mp, "Error to update target - memory allocation");; my_error_msg = apr_psprintf(ruleset->mp, "Error to update target - memory allocation");
goto end;
}
if (p3 != NULL) { if (p3 != NULL) {
replace = strdup(p3); replace = strdup(p3);
if (replace == NULL) { if (replace == NULL) {
free(target_list); my_error_msg = apr_psprintf(ruleset->mp, "Error to update target - memory allocation");
target_list = NULL; goto end;
return apr_psprintf(ruleset->mp, "Error to update target - memory allocation");;
} }
} }
if (replace != NULL) { if (replace != NULL) {
opt = strchr(replace,'!'); opt = strchr(replace,'!');
if (opt != NULL) { if (opt != NULL) {
*opt = '\0'; *opt = '\0';
opt++; opt++;
@@ -295,29 +293,13 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
} }
if (apr_table_get(ruleset->engine->variables, name) == NULL) { if (apr_table_get(ruleset->engine->variables, name) == NULL) {
if(target_list != NULL) my_error_msg = apr_psprintf(ruleset->mp, "Error to update target - [%s] is not valid target", name);
free(target_list); goto end;
if(replace != NULL)
free(replace);
if(msr) {
msr_log(msr, 9, "Error to update target - [%s] is not valid target", name);
}
return apr_psprintf(ruleset->mp, "Error to update target - [%s] is not valid target", name);
} }
name_len = strlen(name); name_len = strlen(name);
if(value != NULL) if (value != NULL) value_len = strlen(value);
value_len = strlen(value);
if(msr) {
msr_log(msr, 9, "Trying to replace by variable name [%s] value [%s]", name, value);
}
#if !defined(MSC_TEST)
else {
ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, " ModSecurity: Trying to replace by variable name [%s] value [%s]", name, value);
}
#endif
targets = (msre_var **)rule->targets->elts; targets = (msre_var **)rule->targets->elts;
// TODO need a good way to remove the element from array, maybe change array by tables or rings // TODO need a good way to remove the element from array, maybe change array by tables or rings
@@ -355,14 +337,8 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
if (match == 1) { if (match == 1) {
rc = msre_parse_targets(ruleset, p, rule->targets, &my_error_msg); rc = msre_parse_targets(ruleset, p, rule->targets, &my_error_msg);
if (rc < 0) { if (rc < 0) {
if(msr) { if (my_error_msg) my_error_msg = apr_psprintf(ruleset->mp, "Error parsing rule targets to replace variable: %s", my_error_msg);
msr_log(msr, 9, "Error parsing rule targets to replace variable"); else my_error_msg = apr_psprintf(ruleset->mp, "Error parsing rule targets to replace variable");
}
#if !defined(MSC_TEST)
else {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Error parsing rule targets to replace variable");
}
#endif
goto end; goto end;
} }
if (msr) { if (msr) {
@@ -376,21 +352,17 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
var_appended = 1; var_appended = 1;
} else { } else {
if(msr) { my_error_msg = apr_psprintf(ruleset->mp, "Cannot find variable to replace");
msr_log(msr, 9, "Cannot find variable to replace");
}
#if !defined(MSC_TEST)
else {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Cannot find varibale to replace");
}
#endif
goto end; goto end;
} }
} else { }
else {
target = strdup(p); target = strdup(p);
if(target == NULL) if (target == NULL) {
return NULL; my_error_msg = apr_psprintf(ruleset->mp, "Error to update target - memory allocation");
goto end;
}
is_negated = is_counting = 0; is_negated = is_counting = 0;
param = name = value = NULL; param = name = value = NULL;
@@ -412,7 +384,6 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
} }
opt = strchr(param,':'); opt = strchr(param,':');
if (opt != NULL) { if (opt != NULL) {
name = apr_strtok(param,":",&value); name = apr_strtok(param,":",&value);
} else { } else {
@@ -420,20 +391,13 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
} }
if (apr_table_get(ruleset->engine->variables, name) == NULL) { if (apr_table_get(ruleset->engine->variables, name) == NULL) {
if(target_list != NULL) my_error_msg = apr_psprintf(ruleset->mp, "Error to update target - [%s] is not valid target", name);
free(target_list); goto end;
if(replace != NULL)
free(replace);
if(msr) {
msr_log(msr, 9, "Error to update target - [%s] is not valid target", name);
}
return apr_psprintf(ruleset->mp, "Error to update target - [%s] is not valid target", name);
} }
name_len = strlen(name); name_len = strlen(name);
if(value != NULL) if (value != NULL) value_len = strlen(value);
value_len = strlen(value);
if (msr) { if (msr) {
msr_log(msr, 9, "Trying to append variable name [%s] value [%s]", name, value); msr_log(msr, 9, "Trying to append variable name [%s] value [%s]", name, value);
@@ -459,8 +423,7 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
} }
} else if (value == NULL && targets[i]->param == NULL){ } else if (value == NULL && targets[i]->param == NULL){
match = 1; match = 1;
} else } else continue;
continue;
} }
} }
@@ -473,14 +436,7 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
if (match == 0 ) { if (match == 0 ) {
rc = msre_parse_targets(ruleset, p, rule->targets, &my_error_msg); rc = msre_parse_targets(ruleset, p, rule->targets, &my_error_msg);
if (rc < 0) { if (rc < 0) {
if(msr) { my_error_msg = apr_psprintf(ruleset->mp, "Error parsing rule targets to append variable");
msr_log(msr, 9, "Error parsing rule targets to append variable");
}
#if !defined(MSC_TEST)
else {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Error parsing rule targets to append variable");
}
#endif
goto end; goto end;
} }
var_appended = 1; var_appended = 1;
@@ -515,19 +471,14 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r
} }
end: end:
if(target_list != NULL) { if (my_error_msg) {
free(target_list); if (msr) msr_log(msr, 9, my_error_msg);
target_list = NULL; else ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, my_error_msg);
} }
if(replace != NULL) { if (target_list != NULL) free(target_list);
free(replace); if (replace != NULL) free(replace);
replace = NULL; if (target != NULL) free(target);
} return my_error_msg;
if(target != NULL) {
free(target);
target = NULL;
}
return NULL;
} }
int msre_ruleset_rule_matches_exception(msre_rule *rule, rule_exception *re) { int msre_ruleset_rule_matches_exception(msre_rule *rule, rule_exception *re) {