diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 091aa0e0..0ee72865 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -867,10 +867,7 @@ static int hook_request_early(request_rec *r) { * create the initial configuration. */ msr = create_tx_context(r); - if (msr == NULL) { - msr_log(msr, 9, "Failed to create context after request failure."); - return DECLINED; - } + if (msr == NULL) return DECLINED; if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "Context created after request failure."); } @@ -1165,11 +1162,7 @@ static void hook_error_log(const char *file, int line, int level, apr_status_t s #else msr = create_tx_context((request_rec*)r); #endif - if (msr == NULL) { - msr_log(msr, 9, "Failed to create context after request failure."); - return; - } - if (msr->txcfg->debuglog_level >= 9) { + if (msr && msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "Context created after request failure."); } } diff --git a/apache2/msc_json.c b/apache2/msc_json.c index b42aa96a..db0f9f02 100644 --- a/apache2/msc_json.c +++ b/apache2/msc_json.c @@ -87,6 +87,7 @@ int json_add_argument(modsec_rec *msr, const char *value, unsigned length) static int yajl_map_key(void *ctx, const unsigned char *key, size_t length) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); unsigned char *safe_key = (unsigned char *) NULL; /** @@ -118,6 +119,7 @@ static int yajl_map_key(void *ctx, const unsigned char *key, size_t length) static int yajl_null(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); return json_add_argument(msr, "", 0); } @@ -128,6 +130,7 @@ static int yajl_null(void *ctx) static int yajl_boolean(void *ctx, int value) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); if (value) { return json_add_argument(msr, "true", strlen("true")); @@ -143,6 +146,7 @@ static int yajl_boolean(void *ctx, int value) static int yajl_string(void *ctx, const unsigned char *value, size_t length) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); return json_add_argument(msr, value, length); } @@ -155,12 +159,14 @@ static int yajl_string(void *ctx, const unsigned char *value, size_t length) static int yajl_number(void *ctx, const char *value, size_t length) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); return json_add_argument(msr, value, length); } static int yajl_start_array(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); if (!msr->json->current_key && !msr->json->prefix) { msr->json->prefix = apr_pstrdup(msr->mp, "array"); @@ -190,6 +196,7 @@ static int yajl_start_array(void *ctx) { static int yajl_end_array(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); unsigned char *separator = (unsigned char *) NULL; /** @@ -226,6 +233,7 @@ static int yajl_end_array(void *ctx) { static int yajl_start_map(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); /** * If we do not have a current_key, this is a top-level hash, so we do not @@ -264,6 +272,7 @@ static int yajl_start_map(void *ctx) static int yajl_end_map(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; + assert(msr != NULL); unsigned char *separator = (unsigned char *) NULL; /** diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index e40136b4..7d56dd64 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -24,9 +24,6 @@ void validate_quotes(modsec_rec *msr, char *data, char quote) { assert(msr != NULL); int i, len; - if(msr == NULL) - return; - if(msr->mpd == NULL) return; diff --git a/apache2/re.c b/apache2/re.c index 77ea8ead..816b911d 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -215,11 +215,9 @@ char *msre_ruleset_phase_rule_update_target_matching_exception(modsec_rec *msr, rules = (msre_rule **)phase_arr->elts; for (i = 0; i < phase_arr->nelts; i++) { msre_rule *rule = (msre_rule *)rules[i]; - if (mode == 0) { /* Looking for next rule. */ if (msre_ruleset_rule_matches_exception(rule, re)) { - - err = update_rule_target_ex(NULL, ruleset, rule, p2, p3); + err = update_rule_target_ex(msr, ruleset, rule, p2, p3); if (err) return err; if (rule->actionset->is_chained) mode = 2; /* Match all rules in this chain. */ } else { @@ -3141,6 +3139,8 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { /* Perform transformations. */ tarr = apr_table_elts(normtab); + /* if no transformation, multi_match makes no sense and breaks the logic */ + if (tarr->nelts == 0) multi_match = 0; /* Execute transformations in a loop. */ diff --git a/design.md b/design.md new file mode 100644 index 00000000..a8e56e12 --- /dev/null +++ b/design.md @@ -0,0 +1,29 @@ +Design notes for source code +== +This file give some explanations and guidelines regarding ModSecurity v2 source code. +The goal is to discuss topics that are not related to a specific location in the code, so that cannot be best explained by comments. +The goal is not to replace comments where it is probably better. +It's quite short for the moment, but the goal is to extend it from time to time. + +## Null pointer check +The default behaviour is to check for null pointer dereference everywhere it may be needed. +In case a pointer cannot be null, it has to be explained with a comment at the beginning of the function of when dereferencing the pointer. +On top of that, an explicit check should be done when compiling in debug mode with the following code: +``` + assert(mypointer); +``` +In case a pointer that cannot be null is used at several locations (say more than 3 times), +the explanation could be given globally in this file. + +### Pointers never null +The following pointers can never be null: + +#### msr + +msr is assigned at the following places: +- mod_security2.c (14 x): initialization +In all the above calls, and all calling functions, it immediately returns (with an error code) in case msr is null, up to a place where no mod_security2 processing at all occurs. +In subsequent calls, there's thus no possibility to have msr null. +- apache2_io.c (2 x): assign a previously initialized msr +- msc_json (9 x): assign a previously initialized msr +- msc_lua.c (4 x): assign a previously initialized msr