Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak that occurs on JSON parsing error #2236

Conversation

vkrivopalov
Copy link

ModSecurity uses a dynamically allocated error message when JSON parsing
fails but never releases it properly.

Signed-off-by: Vladimir Krivopalov [email protected]

ModSecurity uses a dynamically allocated error message when JSON parsing
fails but never releases it properly.

Signed-off-by: Vladimir Krivopalov <[email protected]>
@zimmerle zimmerle added the 2.x Related to ModSecurity version 2.x label Jan 14, 2020
@zimmerle zimmerle self-requested a review February 4, 2020 14:01
@zimmerle zimmerle self-assigned this Feb 4, 2020
@zimmerle
Copy link
Contributor

zimmerle commented Feb 4, 2020

Thank you for the contribution @argenet.

@marcstern
Copy link

We're using this in prod from Feb. in dozens of environments

@vloup
Copy link

vloup commented Nov 29, 2021

This PR cannot get merged since release v2.9.5.

This is my suggested rebase:

diff --git a/apache2/msc_json.c b/apache2/msc_json.c
index d69e9eb7..7d76dc05 100644
--- a/apache2/msc_json.c
+++ b/apache2/msc_json.c
@@ -355,7 +355,9 @@ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
        if (msr->json->depth_limit_exceeded) {
            *error_msg = "JSON depth limit exceeded";
        } else {
-           *error_msg = yajl_get_error(msr->json->handle, 0, NULL, 0);
+           char *yajl_err = yajl_get_error(msr->json->handle, 0, buf, size);
+           *error_msg = apr_pstrdup(msr->mp, yajl_err);
+           yajl_free_error(msr->json->handle, yajl_err);
        }
         return -1;
     }
@@ -379,7 +381,9 @@ int json_complete(modsec_rec *msr, char **error_msg) {
        if (msr->json->depth_limit_exceeded) {
            *error_msg = "JSON depth limit exceeded";
        } else {
-           *error_msg = yajl_get_error(msr->json->handle, 0, NULL, 0);
+           char *yajl_err = yajl_get_error(msr->json->handle, 0, NULL, 0);
+           *error_msg = apr_pstrdup(msr->mp, yajl_err);
+           yajl_free_error(msr->json->handle, yajl_err);
        }
 
         return -1;

What do you think @argenet ?

@martinhsv
Copy link
Contributor

Replaced by equivalent #2663 and merged.

Thanks all.

@martinhsv martinhsv closed this Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to ModSecurity version 2.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants