From 9fb773c1ce0f0cad5a3b83ebef3c021bb9043f39 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Sat, 20 Jul 2024 18:45:14 +0200 Subject: [PATCH 01/20] Invalid pointer access in case rule id == NOT_SET_P --- apache2/apache2_config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index c0464f50..78797ae4 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -35,7 +35,7 @@ const char* id_log(msre_rule* rule) { assert(rule != NULL); assert(rule->actionset != NULL); const char* id = rule->actionset->id; - if (!id || !*id || id == NOT_SET_P) id = apr_psprintf(rule->ruleset->mp, "%s (%d)", rule->filename, rule->line_num); + if (!id || id == NOT_SET_P || !*id) id = apr_psprintf(rule->ruleset->mp, "%s (%d)", rule->filename, rule->line_num); return id; } From ca593a4a401308bb12749fa52cc93d2b2ddff395 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Sat, 20 Jul 2024 18:53:30 +0200 Subject: [PATCH 02/20] Passing address of lock instead of lock in acquire_global_lock() --- apache2/modsecurity.c | 25 ++++++++++--------------- apache2/modsecurity.h | 2 +- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index 74b6eca9..2990e407 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -122,7 +122,7 @@ msc_engine *modsecurity_create(apr_pool_t *mp, int processing_mode) { return msce; } -int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp) { +int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp) { apr_status_t rc; apr_file_t *lock_name; const char *temp_dir; @@ -146,7 +146,7 @@ int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp) { // below func always return APR_SUCCESS apr_file_name_get(&filename, lock_name); - rc = apr_global_mutex_create(&lock, filename, APR_LOCK_DEFAULT, mp); + rc = apr_global_mutex_create(lock, filename, APR_LOCK_DEFAULT, mp); if (rc != APR_SUCCESS) { ap_log_perror(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Could not create global mutex"); return -1; @@ -154,11 +154,12 @@ int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp) { #if !defined(MSC_TEST) #ifdef __SET_MUTEX_PERMS #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2 - rc = ap_unixd_set_global_mutex_perms(lock); + rc = ap_unixd_set_global_mutex_perms(*lock); #else - rc = unixd_set_global_mutex_perms(lock); + rc = unixd_set_global_mutex_perms(*lock); #endif if (rc != APR_SUCCESS) { + ap_log_perror(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Could not set permissions on global mutex"); return -1; } #endif /* SET_MUTEX_PERMS */ @@ -189,21 +190,15 @@ int modsecurity_init(msc_engine *msce, apr_pool_t *mp) { curl_global_init(CURL_GLOBAL_ALL); #endif /* Serial audit log mutex */ - rc = acquire_global_lock(msce->auditlog_lock, mp); - if (rc != APR_SUCCESS) { - return -1; - } + rc = acquire_global_lock(&msce->auditlog_lock, mp); + if (rc != APR_SUCCESS) return -1; - rc = acquire_global_lock(msce->geo_lock, mp); - if (rc != APR_SUCCESS) { - return -1; - } + rc = acquire_global_lock(&msce->geo_lock, mp); + if (rc != APR_SUCCESS) return -1; #ifdef GLOBAL_COLLECTION_LOCK rc = acquire_global_lock(&msce->dbm_lock, mp); - if (rc != APR_SUCCESS) { - return -1; - } + if (rc != APR_SUCCESS) return -1; #endif /* GLOBAL_COLLECTION_LOCK */ return 1; diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 143a8231..d1aa1d83 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -706,7 +706,7 @@ struct msc_parm { }; /* Reusable functions */ -int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp); +int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp); /* Engine functions */ From f32be70793ffc2374875d0646f0ee715b06af00f Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Mon, 22 Jul 2024 16:24:56 +0200 Subject: [PATCH 03/20] Use standard httpd logging format in error log --- apache2/mod_security2.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 0ee72865..1850191e 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -103,17 +103,17 @@ static int server_limit, thread_limit; * * \param mp Pointer to memory pool */ -static void version(apr_pool_t *mp) { +static void version(apr_pool_t *mp, server_rec* s) { char *pcre_vrs = NULL; const char *pcre_loaded_vrs = NULL; char pcre2_loaded_vrs_buffer[80] ={0}; - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: APR compiled version=\"%s\"; " "loaded version=\"%s\"", APR_VERSION_STRING, apr_version_string()); if (strstr(apr_version_string(), APR_VERSION_STRING) == NULL) { - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, "ModSecurity: Loaded APR do not match with compiled!"); + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, "ModSecurity: Loaded APR do not match with compiled!"); } #ifdef WITH_PCRE2 @@ -134,21 +134,21 @@ static void version(apr_pool_t *mp) { "loaded version=\"%s\"", pcre_vrs, pcre_loaded_vrs); if (strstr(pcre_loaded_vrs,pcre_vrs) == NULL) { - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, "ModSecurity: Loaded PCRE do not match with compiled!"); + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, "ModSecurity: Loaded PCRE do not match with compiled!"); } /* Lua version function was removed in current 5.1. Need to check in future versions if it's back */ #if defined(WITH_LUA) - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: LUA compiled version=\"%s\"", LUA_VERSION); #endif /* WITH_LUA */ #ifdef WITH_YAJL - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: YAJL compiled version=\"%d.%d.%d\"", YAJL_MAJOR, YAJL_MINOR, YAJL_MICRO); #endif /* WITH_YAJL */ - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: LIBXML compiled version=\"%s\"", LIBXML_DOTTED_VERSION); } @@ -778,7 +778,7 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t ap_log_error(APLOG_MARK, APLOG_NOTICE | APLOG_NOERRNO, 0, s, "%s configured.", MODSEC_MODULE_NAME_FULL); - version(mp); + version(mp, s); /* If we've changed the server signature make note of the original. */ if (new_server_signature != NULL) { From cd65a44d64d61c84f20207e8a618f93c9ce47178 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Mon, 22 Jul 2024 16:53:58 +0200 Subject: [PATCH 04/20] Removed useless code --- apache2/apache2_io.c | 2 -- apache2/msc_crypt.c | 8 ++--- apache2/msc_reqbody.c | 15 +++----- apache2/msc_util.c | 24 ++++--------- apache2/re.c | 6 ++-- apache2/re_actions.c | 77 ++++++++++++++++++++---------------------- apache2/re_operators.c | 34 +++++-------------- apache2/re_variables.c | 2 +- 8 files changed, 63 insertions(+), 105 deletions(-) diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index 5d2ef85b..405b649a 100644 --- a/apache2/apache2_io.c +++ b/apache2/apache2_io.c @@ -629,7 +629,6 @@ static int flatten_response_body(modsec_rec *msr) { return -1; } - memset(msr->stream_output_data, 0, msr->stream_output_length+1); memcpy(msr->stream_output_data, msr->resbody_data, msr->stream_output_length); msr->stream_output_data[msr->stream_output_length] = '\0'; } else if (msr->txcfg->stream_outbody_inspection && msr->txcfg->hash_is_enabled == HASH_ENABLED) { @@ -662,7 +661,6 @@ static int flatten_response_body(modsec_rec *msr) { return -1; } - memset(msr->stream_output_data, 0, msr->stream_output_length+1); memcpy(msr->stream_output_data, msr->resbody_data, msr->stream_output_length); msr->stream_output_data[msr->stream_output_length] = '\0'; } diff --git a/apache2/msc_crypt.c b/apache2/msc_crypt.c index 3287eeff..4f73d15a 100644 --- a/apache2/msc_crypt.c +++ b/apache2/msc_crypt.c @@ -1156,8 +1156,8 @@ int inject_hashed_response_body(modsec_rec *msr, int elts) { return -1; } - memset(msr->stream_output_data, 0x0, msr->stream_output_length+1); memcpy(msr->stream_output_data, xmlOutputBufferGetContent(output_buf), msr->stream_output_length); + msr->stream_output_data[msr->stream_output_length] = '\0'; if (msr->txcfg->debuglog_level >= 4) msr_log(msr, 4, "inject_hashed_response_body: Copying XML tree from CONTENT to stream buffer [%zu] bytes.", xmlOutputBufferGetSize(output_buf)); @@ -1187,8 +1187,8 @@ int inject_hashed_response_body(modsec_rec *msr, int elts) { return -1; } - memset(msr->stream_output_data, 0x0, msr->stream_output_length+1); memcpy(msr->stream_output_data, xmlOutputBufferGetContent(output_buf), msr->stream_output_length); + msr->stream_output_data[msr->stream_output_length] = '\0'; if (msr->txcfg->debuglog_level >= 4) msr_log(msr, 4, "inject_hashed_response_body: Copying XML tree from CONV to stream buffer [%zu] bytes.", xmlOutputBufferGetSize(output_buf)); @@ -1222,9 +1222,9 @@ int inject_hashed_response_body(modsec_rec *msr, int elts) { return -1; } - memset(msr->stream_output_data, 0x0, msr->stream_output_length+1); memcpy(msr->stream_output_data, (char *)xmlBufferContent(output_buf->buffer), msr->stream_output_length); //memcpy(msr->stream_output_data, output_buf->buffer->content, msr->stream_output_length); + msr->stream_output_data[msr->stream_output_length] = '\0'; if (msr->txcfg->debuglog_level >= 4) msr_log(msr, 4, "inject_hashed_response_body: Copying XML tree from CONTENT to stream buffer [%d] bytes.", msr->stream_output_length); @@ -1254,9 +1254,9 @@ int inject_hashed_response_body(modsec_rec *msr, int elts) { return -1; } - memset(msr->stream_output_data, 0x0, msr->stream_output_length+1); memcpy(msr->stream_output_data, (char *)xmlBufferContent(output_buf->conv), msr->stream_output_length); //memcpy(msr->stream_output_data, output_buf->conv->content, msr->stream_output_length); + msr->stream_output_data[msr->stream_output_length] = '\0'; if (msr->txcfg->debuglog_level >= 4) msr_log(msr, 4, "inject_hashed_response_body: Copying XML tree from CONV to stream buffer [%d] bytes.", msr->stream_output_length); diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index ba8bdfd4..c7c081fb 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -461,8 +461,8 @@ apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buf if(data == NULL) return -1; - memset(data, 0, msr->stream_input_length + 1 - buflen); memcpy(data, msr->stream_input_data, msr->stream_input_length - buflen); + data[msr->stream_input_length - buflen] = '\0'; stream_input_body = (char *)realloc(msr->stream_input_data, msr->stream_input_length + 1); @@ -470,28 +470,21 @@ apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buf } if (msr->stream_input_data == NULL) { - if(data) { - free(data); - data = NULL; - } + if (data) free(data); *error_msg = apr_psprintf(msr->mp, "Unable to allocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.", msr->stream_input_length + 1); return -1; } - memset(msr->stream_input_data, 0, msr->stream_input_length+1); - if(first_pkt) { memcpy(msr->stream_input_data, buffer, msr->stream_input_length); } else { memcpy(msr->stream_input_data, data, msr->stream_input_length - buflen); memcpy(msr->stream_input_data+(msr->stream_input_length - buflen), buffer, buflen); } + msr->stream_input_data[msr->stream_input_length] = '\0'; - if(data) { - free(data); - data = NULL; - } + if (data) free(data); #else if (msr->stream_input_data == NULL) { // Is the request body length known beforehand? (requests that are not Transfer-Encoding: chunked) diff --git a/apache2/msc_util.c b/apache2/msc_util.c index fd318a08..bce5b24f 100644 --- a/apache2/msc_util.c +++ b/apache2/msc_util.c @@ -2473,28 +2473,16 @@ not_enough_memory: int read_line(char *buf, int len, FILE *fp) { - char *tmp; + if (buf == NULL) return -1; - if (buf == NULL) - { - return -1; - } - - memset(buf, '\0', len*sizeof(char)); - - if (fgets(buf, len, fp) == NULL) - { + if (fgets(buf, len, fp) == NULL) { *buf = '\0'; return 0; } - else - { - if ((tmp = strrchr(buf, '\n')) != NULL) - { - *tmp = '\0'; - } - } - + + char* tmp; + if ((tmp = strrchr(buf, '\n')) != NULL) *tmp = '\0'; + return 1; } diff --git a/apache2/re.c b/apache2/re.c index 8e69f5ba..6476a94f 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -326,14 +326,14 @@ char *update_rule_target_ex(modsec_rec *msr, msre_ruleset *ruleset, msre_rule *r if(value != NULL && targets[i]->param != NULL) { if((strlen(targets[i]->param) == strlen(value)) && strncasecmp(targets[i]->param,value,strlen(targets[i]->param)) == 0) { - memset(targets[i]->name,0,strlen(targets[i]->name)); - memset(targets[i]->param,0,strlen(targets[i]->param)); + targets[i]->name[0] = '\0'; + targets[i]->param[0] = '\0'; targets[i]->is_counting = 0; targets[i]->is_negated = 1; match = 1; } } else if (value == NULL && targets[i]->param == NULL){ - memset(targets[i]->name,0,strlen(targets[i]->name)); + targets[i]->name[0] = '\0'; targets[i]->is_counting = 0; targets[i]->is_negated = 1; match = 1; diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 36f898dd..c5cdd453 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -1251,19 +1251,19 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, return -1; } - re = apr_pcalloc(msr->mp, sizeof(rule_exception)); - if (re == NULL) { - msr_log(msr, 1, "Ctl: Memory allocation error"); - return -1; - } - re->type = RULE_EXCEPTION_REMOVE_ID; - re->param = (const char *)apr_pstrdup(msr->mp, p1); - if (re->param == NULL) { - msr_log(msr, 1, "Ctl: Memory allocation error"); - return -1; - } - apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); - return 1; + re = apr_pcalloc(msr->mp, sizeof(rule_exception)); + if (re == NULL) { + msr_log(msr, 1, "Ctl: Memory allocation error"); + return -1; + } + re->type = RULE_EXCEPTION_REMOVE_ID; + re->param = (const char *)apr_pstrdup(msr->mp, p1); + if (re->param == NULL) { + msr_log(msr, 1, "Ctl: Memory allocation error"); + return -1; + } + apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); + return 1; } else if (strcasecmp(name, "ruleRemoveTargetByTag") == 0) { rule_exception *re = NULL; @@ -1271,7 +1271,6 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, char *savedptr = NULL; p1 = apr_strtok(value,";",&savedptr); - p2 = apr_strtok(NULL,";",&savedptr); if (msr->txcfg->debuglog_level >= 4) { @@ -1282,16 +1281,16 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, return -1; } - re = apr_pcalloc(msr->mp, sizeof(rule_exception)); - re->type = RULE_EXCEPTION_REMOVE_TAG; - re->param = (const char *)apr_pstrdup(msr->mp, p1); - re->param_data = msc_pregcomp(msr->mp, p1, 0, NULL, NULL); - if (re->param_data == NULL) { - msr_log(msr, 1, "ModSecurity: Invalid regular expression \"%s\"", p1); - return -1; - } - apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); - return 1; + re = apr_pcalloc(msr->mp, sizeof(rule_exception)); + re->type = RULE_EXCEPTION_REMOVE_TAG; + re->param = (const char *)apr_pstrdup(msr->mp, p1); + re->param_data = msc_pregcomp(msr->mp, p1, 0, NULL, NULL); + if (re->param_data == NULL) { + msr_log(msr, 1, "ModSecurity: Invalid regular expression \"%s\"", p1); + return -1; + } + apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); + return 1; } else if (strcasecmp(name, "ruleRemoveTargetByMsg") == 0) { rule_exception *re = NULL; @@ -1299,7 +1298,6 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, char *savedptr = NULL; p1 = apr_strtok(value,";",&savedptr); - p2 = apr_strtok(NULL,";",&savedptr); if (msr->txcfg->debuglog_level >= 4) { @@ -1310,23 +1308,20 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, return -1; } - re = apr_pcalloc(msr->mp, sizeof(rule_exception)); - re->type = RULE_EXCEPTION_REMOVE_MSG; - re->param = apr_pstrdup(msr->mp, p1); - re->param_data = msc_pregcomp(msr->mp, p1, 0, NULL, NULL); - if (re->param_data == NULL) { - msr_log(msr, 1, "ModSecurity: Invalid regular expression \"%s\"", p1); - return -1; - } - apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); - return 1; - } - else { - /* Should never happen, but log if it does. */ - msr_log(msr, 1, "Internal Error: Unknown ctl action \"%s\".", name); - return -1; + re = apr_pcalloc(msr->mp, sizeof(rule_exception)); + re->type = RULE_EXCEPTION_REMOVE_MSG; + re->param = apr_pstrdup(msr->mp, p1); + re->param_data = msc_pregcomp(msr->mp, p1, 0, NULL, NULL); + if (re->param_data == NULL) { + msr_log(msr, 1, "ModSecurity: Invalid regular expression \"%s\"", p1); + return -1; + } + apr_table_addn(msr->removed_targets, apr_pstrdup(msr->mp, p2), (void *)re); + return 1; } + /* Should never happen, but log if it does. */ + msr_log(msr, 1, "Internal Error: Unknown ctl action \"%s\".", name); return -1; } @@ -1764,7 +1759,7 @@ static apr_status_t msre_action_setvar_parse(modsec_rec *msr, apr_pool_t *mptmp, var_value = s + 1; *s = '\0'; - while ((*var_value != '\0')&&(isspace(*var_value))) var_value++; + while (isspace(*var_value)) var_value++; } return msre_action_setvar_execute(msr,mptmp,rule,var_name,var_value); diff --git a/apache2/re_operators.c b/apache2/re_operators.c index 1d812263..30176c51 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -617,24 +617,15 @@ nextround: size+=sl; *data_out=0; - if(msr->stream_output_data != NULL && output_body == 1) { - - memset(msr->stream_output_data, 0x0, msr->stream_output_length); + if (msr->stream_output_data != NULL && output_body == 1) { free(msr->stream_output_data); msr->stream_output_data = NULL; msr->stream_output_length = 0; - msr->stream_output_data = (char *)malloc(size+1); - - if(msr->stream_output_data == NULL) { - return -1; - } + if (msr->stream_output_data == NULL) return -1; msr->stream_output_length = size; - memset(msr->stream_output_data, 0x0, size+1); - msr->of_stream_changed = 1; - memcpy(msr->stream_output_data, data, size); msr->stream_output_data[size] = '\0'; @@ -642,8 +633,7 @@ nextround: var->value = msr->stream_output_data; } - if(msr->stream_input_data != NULL && input_body == 1) { - memset(msr->stream_input_data, 0x0, msr->stream_input_length); + if (msr->stream_input_data != NULL && input_body == 1) { free(msr->stream_input_data); msr->stream_input_data = NULL; msr->stream_input_length = 0; @@ -651,9 +641,7 @@ nextround: msr->stream_input_allocated_length = 0; #endif msr->stream_input_data = (char *)malloc(size+1); - if(msr->stream_input_data == NULL) { - return -1; - } + if(msr->stream_input_data == NULL) return -1; msr->stream_input_length = size; #ifdef MSC_LARGE_STREAM_INPUT @@ -1573,12 +1561,11 @@ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int l int match = 0; url = apr_palloc(pool, len + 1); + if (!url) return NULL; data = apr_palloc(pool, len + 1); + if (!data) return NULL; - memset(data, 0, len+1); - memset(url, 0, len+1); - - memcpy(url, domain, len); + url[len] = 0; while(( pos = strstr(url , "/./" )) != NULL) { match = 1; @@ -1589,8 +1576,7 @@ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int l strncpy(url , data, len); } - if(match == 0) - return domain; + if (match == 0) return domain; return url; } @@ -1681,8 +1667,6 @@ static int verify_gsb(gsb_db *gsb, modsec_rec *msr, const char *match, unsigned const char *hash = NULL; const char *search = NULL; - memset(digest, 0, sizeof(digest)); - apr_md5_init(&ctx); if ((rc = apr_md5_update(&ctx, match, match_length)) != APR_SUCCESS) @@ -1690,7 +1674,7 @@ static int verify_gsb(gsb_db *gsb, modsec_rec *msr, const char *match, unsigned apr_md5_final(digest, &ctx); - hash = apr_psprintf(msr->mp, "%s", bytes2hex(msr->mp, digest, 16)); + hash = apr_psprintf(msr->mp, "%s", bytes2hex(msr->mp, digest, APR_MD5_DIGESTSIZE)); if ((hash != NULL) && (gsb->gsb_table != NULL)) { search = apr_hash_get(gsb->gsb_table, hash, APR_HASH_KEY_STRING); diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 5aa7589a..dee23af2 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -2491,7 +2491,7 @@ static int var_full_request_generate(modsec_rec *msr, msre_var *var, } goto failed_not_enough_mem; } - memset(full_request, '\0', sizeof(char)*msr->msc_full_request_length); + full_request[0] = '\0'; msr->msc_full_request_buffer = full_request; msr->msc_full_request_length = full_request_length; From 9b987cc3f92d06f50c430732413ee96dc22b681a Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Mon, 22 Jul 2024 17:08:16 +0200 Subject: [PATCH 05/20] Return of msc_regexec() compared with PCRE_ERROR_NOMATCH (!=) to check if match. Other errors may happen that would return -2, -3, ... Matching would be incorrectly set in this case. We must check if >= 0 --- apache2/re_variables.c | 116 +++++++++++++---------------------------- 1 file changed, 35 insertions(+), 81 deletions(-) diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 5aa7589a..c6277593 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -120,8 +120,7 @@ static int var_args_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; /* Run the regex against the argument name. */ - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -198,8 +197,7 @@ static int var_args_names_generate(modsec_rec *msr, msre_var *var, msre_rule *ru else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -250,8 +248,7 @@ static int var_args_get_generate(modsec_rec *msr, msre_var *var, msre_rule *rule if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; /* Run the regex against the argument name. */ - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -300,8 +297,7 @@ static int var_args_get_names_generate(modsec_rec *msr, msre_var *var, msre_rule else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -352,8 +348,7 @@ static int var_args_post_generate(modsec_rec *msr, msre_var *var, msre_rule *rul if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; /* Run the regex against the argument name. */ - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -402,8 +397,7 @@ static int var_args_post_names_generate(modsec_rec *msr, msre_var *var, msre_rul else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, arg->name, - arg->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, arg->name, arg->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(arg->name, var->param) == 0) match = 1; } @@ -899,8 +893,7 @@ static int var_tx_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -955,8 +948,7 @@ static int var_geo_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1016,8 +1008,7 @@ static int var_ip_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1096,8 +1087,7 @@ static int var_session_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1152,8 +1142,7 @@ static int var_user_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1212,8 +1201,7 @@ static int var_global_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1264,8 +1252,7 @@ static int var_resource_generate(modsec_rec *msr, msre_var *var, msre_rule *rule else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - str->name_len, &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, str->name_len, &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -1306,38 +1293,21 @@ static int var_files_tmp_contents_generate(modsec_rec *msr, msre_var *var, if (msr->mpd == NULL) return 0; parts = (multipart_part **)msr->mpd->parts->elts; - for (i = 0; i < msr->mpd->parts->nelts; i++) - { - if ((parts[i]->type == MULTIPART_FILE) && - (parts[i]->tmp_file_name != NULL)) - { + for (i = 0; i < msr->mpd->parts->nelts; i++) { + if ((parts[i]->type == MULTIPART_FILE) && (parts[i]->tmp_file_name != NULL)) { int match = 0; /* Figure out if we want to include this variable. */ - if (var->param == NULL) - { - match = 1; - } - else - { - if (var->param_data != NULL) - { + if (var->param == NULL)match = 1; + else { + if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, - parts[i]->name, strlen(parts[i]->name), - &my_error_msg) == PCRE_ERROR_NOMATCH)) - { - match = 1; - } + if (msc_regexec((msc_regex_t*)var->param_data, parts[i]->name, strlen(parts[i]->name), &my_error_msg) >= 0) match = 1; } - else - { + else { /* Simple comparison. */ - if (strcasecmp(parts[i]->name, var->param) == 0) - { - match = 1; - } + if (strcasecmp(parts[i]->name, var->param) == 0)match = 1; } } /* If we had a match add this argument to the collection. */ @@ -1351,10 +1321,7 @@ static int var_files_tmp_contents_generate(modsec_rec *msr, msre_var *var, msre_var *rvar = NULL; file = fopen(parts[i]->tmp_file_name, "r"); - if (file == NULL) - { - continue; - } + if (file == NULL) continue; full_content = (char *)apr_pcalloc(mptmp, (sizeof(char)*parts[i]->length) + 1); if (full_content == NULL) { @@ -1416,8 +1383,7 @@ static int var_files_tmpnames_generate(modsec_rec *msr, msre_var *var, msre_rule else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, - strlen(parts[i]->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, strlen(parts[i]->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(parts[i]->name, var->param) == 0) match = 1; } @@ -1465,8 +1431,7 @@ static int var_files_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, - strlen(parts[i]->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, strlen(parts[i]->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(parts[i]->name, var->param) == 0) match = 1; } @@ -1514,8 +1479,7 @@ static int var_files_sizes_generate(modsec_rec *msr, msre_var *var, msre_rule *r else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, - strlen(parts[i]->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, strlen(parts[i]->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(parts[i]->name, var->param) == 0) match = 1; } @@ -1632,8 +1596,7 @@ static int var_multipart_part_headers_generate(modsec_rec *msr, msre_var *var, m else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, - strlen(parts[i]->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, parts[i]->name, strlen(parts[i]->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(parts[i]->name, var->param) == 0) match = 1; } @@ -2105,8 +2068,7 @@ static int var_perf_rules_generate(modsec_rec *msr, msre_var *var, msre_rule *ru else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -2591,8 +2553,7 @@ static int var_matched_vars_names_generate(modsec_rec *msr, msre_var *var, msre_ else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - strlen(str->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, strlen(str->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -2660,8 +2621,7 @@ static int var_matched_vars_generate(modsec_rec *msr, msre_var *var, msre_rule * else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, str->name, - strlen(str->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, str->name, strlen(str->name), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(str->name, var->param) == 0) match = 1; } @@ -2728,8 +2688,7 @@ static int var_request_cookies_generate(modsec_rec *msr, msre_var *var, msre_rul else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -2783,8 +2742,7 @@ static int var_request_cookies_names_generate(modsec_rec *msr, msre_var *var, ms else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -2838,8 +2796,7 @@ static int var_request_headers_generate(modsec_rec *msr, msre_var *var, msre_rul else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -2893,8 +2850,7 @@ static int var_request_headers_names_generate(modsec_rec *msr, msre_var *var, ms else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -3171,8 +3127,7 @@ static int var_response_headers_generate(modsec_rec *msr, msre_var *var, msre_ru else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } @@ -3226,8 +3181,7 @@ static int var_response_headers_names_generate(modsec_rec *msr, msre_var *var, m else { if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; - if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, - strlen(te[i].key), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1; + if (msc_regexec((msc_regex_t *)var->param_data, te[i].key, strlen(te[i].key), &my_error_msg) >= 0) match = 1; } else { /* Simple comparison. */ if (strcasecmp(te[i].key, var->param) == 0) match = 1; } From f143663cf0ddee6a2458d3d7806fc151a6b5e99c Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 25 Jul 2024 09:30:48 +0200 Subject: [PATCH 06/20] Add collection in log in case of writing error --- apache2/persist_dbm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apache2/persist_dbm.c b/apache2/persist_dbm.c index 6634151c..c685e1de 100644 --- a/apache2/persist_dbm.c +++ b/apache2/persist_dbm.c @@ -638,9 +638,9 @@ int collection_store(modsec_rec *msr, apr_table_t *col) { #endif if (msr->txcfg->debuglog_level >= 4) { - msr_log(msr, 4, "collection_store: Persisted collection (name \"%s\", key \"%s\").", + msr_log(msr, 4, "collection_store: Persisted collection (name \"%s\", key \"%s\", length=%d).", log_escape_ex(msr->mp, var_name->value, var_name->value_len), - log_escape_ex(msr->mp, var_key->value, var_key->value_len)); + log_escape_ex(msr->mp, var_key->value, var_key->value_len), value.dsize); } return 0; From 223ce91aeeb49e59590f4dcc295cacee48cb60c1 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Thu, 25 Jul 2024 20:52:55 +0200 Subject: [PATCH 07/20] Move xmlFree() call to the right place --- apache2/re_variables.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 5aa7589a..a080a863 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -745,13 +745,12 @@ static int var_xml_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, } /* Create one variable for each node in the result. */ + char* content = NULL; for(i = 0; i < nodes->nodeNr; i++) { msre_var *rvar = NULL; - char *content = NULL; content = (char *)xmlNodeGetContent(nodes->nodeTab[i]); if (content != NULL) { - xmlFree(content); rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); if (!rvar) { msr_log(msr, 1, "XML: Memory allocation error"); @@ -766,12 +765,15 @@ static int var_xml_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, } rvar->value_len = strlen(rvar->value); apr_table_addn(vartab, rvar->name, (void *)rvar); + xmlFree(content); + content = NULL; count++; } } var_xml_generate_Error: + if (content != NULL) xmlFree(content); xmlXPathFreeObject(xpathObj); xmlXPathFreeContext(xpathCtx); From 0be1f1566a6a98e48822be0d6bea1a5dfc28ecab Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 31 Jul 2024 09:38:20 +0200 Subject: [PATCH 08/20] Remove redundant entry [client %s] is added by the standard httpd log function => remove it --- apache2/apache2_util.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apache2/apache2_util.c b/apache2/apache2_util.c index 20ea940e..f4243679 100644 --- a/apache2/apache2_util.c +++ b/apache2/apache2_util.c @@ -286,19 +286,16 @@ static void internal_log_ex(request_rec *r, directory_config *dcfg, modsec_rec * #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2 ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, - "[client %s] ModSecurity: %s%s [uri \"%s\"]%s", r->useragent_ip ? r->useragent_ip : r->connection->client_ip, str1, - hostname, log_escape(msr->mp, r->uri), unique_id); + "ModSecurity: %s%s [uri \"%s\"]%s", str1, hostname, log_escape(msr->mp, r->uri), unique_id); #else ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r->server, - "[client %s] ModSecurity: %s%s [uri \"%s\"]%s", msr->remote_addr ? msr->remote_addr : r->connection->remote_ip, str1, - hostname, log_escape(msr->mp, r->uri), unique_id); + "ModSecurity: %s%s [uri \"%s\"]%s", str1, hostname, log_escape(msr->mp, r->uri), unique_id); #endif /* Add this message to the list. */ if (msr != NULL) { /* Force relevency if this is an alert */ msr->is_relevant++; - *(const char **)apr_array_push(msr->alerts) = apr_pstrdup(msr->mp, str1); } } From 7c379c8d5944578284e9baa3849f993168007fd3 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 31 Jul 2024 11:17:36 +0200 Subject: [PATCH 09/20] Fixed assert() usage: - added some missing - removed some invalid - removed some that were not relevant in the context of the current function, when done in a called function --- apache2/apache2_config.c | 378 +++++++++++++++++++++++++++------------ apache2/modsecurity.c | 2 + apache2/msc_json.c | 10 ++ apache2/msc_logging.c | 5 +- apache2/msc_multipart.c | 1 + apache2/msc_parsers.c | 1 + apache2/msc_reqbody.c | 3 + apache2/msc_xml.c | 2 + apache2/re.c | 36 +++- apache2/re_actions.c | 89 +++++++++ apache2/re_operators.c | 76 ++++++-- apache2/re_variables.c | 93 ++++++++-- 12 files changed, 552 insertions(+), 144 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index c0464f50..cc37d8eb 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -34,6 +34,7 @@ const char* id_log(msre_rule* rule) { assert(rule != NULL); assert(rule->actionset != NULL); + assert(rule->ruleset != NULL); const char* id = rule->actionset->id; if (!id || !*id || id == NOT_SET_P) id = apr_psprintf(rule->ruleset->mp, "%s (%d)", rule->filename, rule->line_num); return id; @@ -188,6 +189,9 @@ static void copy_rules_phase(apr_pool_t *mp, apr_array_header_t *child_phase_arr, apr_array_header_t *exceptions_arr) { + assert(parent_phase_arr != NULL); + assert(child_phase_arr != NULL); + assert(exceptions_arr != NULL); rule_exception **exceptions; msre_rule **rules; int i, j; @@ -196,11 +200,14 @@ static void copy_rules_phase(apr_pool_t *mp, rules = (msre_rule **)parent_phase_arr->elts; for(i = 0; i < parent_phase_arr->nelts; i++) { msre_rule *rule = (msre_rule *)rules[i]; + assert(rule != NULL); + assert(rule->actionset != NULL); int copy = 1; if (mode == 0) { /* First rule in the chain. */ exceptions = (rule_exception **)exceptions_arr->elts; + assert(exceptions != NULL); for(j = 0; j < exceptions_arr->nelts; j++) { /* Process exceptions. */ @@ -290,14 +297,11 @@ static int copy_rules(apr_pool_t *mp, msre_ruleset *parent_ruleset, msre_ruleset *child_ruleset, apr_array_header_t *exceptions_arr) { + assert(parent_ruleset != NULL); + assert(child_ruleset != NULL); + assert(exceptions_arr != NULL); int ret = 0; - if (parent_ruleset == NULL || child_ruleset == NULL || - exceptions_arr == NULL) { - ret = -1; - goto failed; - } - copy_rules_phase(mp, parent_ruleset->phase_request_headers, child_ruleset->phase_request_headers, exceptions_arr); copy_rules_phase(mp, parent_ruleset->phase_request_body, @@ -318,6 +322,8 @@ failed: */ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) { + assert(_parent != NULL); + assert(_child != NULL); directory_config *parent = (directory_config *)_parent; directory_config *child = (directory_config *)_child; directory_config *merged = create_directory_config(mp, NULL); @@ -785,11 +791,14 @@ void init_directory_config(directory_config *dcfg) static const char *add_rule(cmd_parms *cmd, directory_config *dcfg, int type, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(dcfg != NULL); char *my_error_msg = NULL; //msre_rule *rule = NULL, *tmp_rule = NULL; char *rid = NULL; msre_rule *rule = NULL; extern msc_engine *modsecurity; + assert(modsecurity != NULL); int type_with_lua = 1; int type_rule; int rule_actionset; @@ -1017,9 +1026,12 @@ static const char *add_rule(cmd_parms *cmd, directory_config *dcfg, int type, static const char *add_marker(cmd_parms *cmd, directory_config *dcfg, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(dcfg != NULL); char *my_error_msg = NULL; msre_rule *rule = NULL; extern msc_engine *modsecurity; + assert(modsecurity != NULL); int p; #ifdef DEBUG_CONF @@ -1068,11 +1080,14 @@ static const char *add_marker(cmd_parms *cmd, directory_config *dcfg, static const char *update_rule_action(cmd_parms *cmd, directory_config *dcfg, const char *p1, const char *p2, int offset) { + assert(cmd != NULL); + assert(dcfg != NULL); char *my_error_msg = NULL; msre_rule *rule = NULL; msre_actionset *new_actionset = NULL; msre_ruleset *ruleset = dcfg->ruleset; extern msc_engine *modsecurity; + assert(modsecurity != NULL); /* Get the ruleset if one exists */ if ((ruleset == NULL)||(ruleset == NOT_SET_P)) { @@ -1151,6 +1166,7 @@ static const char *cmd_action(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_marker(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; const char *action = apr_pstrcat(dcfg->mp, SECMARKER_BASE_ACTIONS, p1, NULL); return add_marker(cmd, (directory_config *)_dcfg, SECMARKER_TARGETS, SECMARKER_ARGS, action); @@ -1159,6 +1175,9 @@ static const char *cmd_marker(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_cookiev0_separator(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; if (strlen(p1) != 1) { @@ -1173,6 +1192,9 @@ static const char *cmd_cookiev0_separator(cmd_parms *cmd, void *_dcfg, static const char *cmd_argument_separator(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; if (strlen(p1) != 1) { @@ -1186,6 +1208,9 @@ static const char *cmd_argument_separator(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_engine(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; if (strcasecmp(p1, "On") == 0) dcfg->auditlog_flag = AUDITLOG_ON; @@ -1202,6 +1227,9 @@ static const char *cmd_audit_engine(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; dcfg->auditlog_name = (char *)p1; @@ -1239,6 +1267,9 @@ static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_audit_log2(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; if (dcfg->auditlog_name == NOT_SET_P) { @@ -1281,6 +1312,9 @@ static const char *cmd_audit_log2(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_audit_log_parts(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; if (is_valid_parts_specification((char *)p1) != 1) { @@ -1294,6 +1328,9 @@ static const char *cmd_audit_log_parts(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_relevant_status(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; #ifdef WITH_PCRE2 @@ -1311,6 +1348,9 @@ static const char *cmd_audit_log_relevant_status(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_type(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; if (strcasecmp(p1, "Serial") == 0) dcfg->auditlog_type = AUDITLOG_SERIAL; @@ -1327,6 +1367,9 @@ static const char *cmd_audit_log_type(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_mode(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; if (strcasecmp(p1, "JSON") == 0) dcfg->auditlog_format = AUDITLOGFORMAT_JSON; @@ -1343,10 +1386,11 @@ static const char *cmd_audit_log_mode(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_dirmode(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "default") == 0) { dcfg->auditlog_dirperms = NOT_SET; } @@ -1365,10 +1409,11 @@ static const char *cmd_audit_log_dirmode(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_filemode(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "default") == 0) { dcfg->auditlog_fileperms = NOT_SET; } @@ -1387,6 +1432,9 @@ static const char *cmd_audit_log_filemode(cmd_parms *cmd, void *_dcfg, static const char *cmd_audit_log_storage_dir(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = _dcfg; dcfg->auditlog_storage_dir = ap_server_root_relative(cmd->pool, p1); @@ -1397,6 +1445,9 @@ static const char *cmd_audit_log_storage_dir(cmd_parms *cmd, void *_dcfg, static const char *cmd_cookie_format(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; if (strcmp(p1, "0") == 0) dcfg->cookie_format = COOKIES_V0; @@ -1411,6 +1462,8 @@ static const char *cmd_cookie_format(cmd_parms *cmd, void *_dcfg, static const char *cmd_chroot_dir(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); char cwd[1025] = ""; if (cmd->server->is_virtual) { @@ -1442,6 +1495,8 @@ static const char *cmd_chroot_dir(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_component_signature(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; /* ENH Enforce "Name/VersionX.Y.Z (comment)" format. */ @@ -1452,14 +1507,17 @@ static const char *cmd_component_signature(cmd_parms *cmd, void *_dcfg, static const char *cmd_content_injection(cmd_parms *cmd, void *_dcfg, int flag) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; dcfg->content_injection_enabled = flag; return NULL; } static const char *cmd_data_dir(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; if (cmd->server->is_virtual) { @@ -1473,6 +1531,9 @@ static const char *cmd_data_dir(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_debug_log(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; apr_status_t rc; @@ -1503,6 +1564,9 @@ static const char *cmd_debug_log(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_collection_timeout(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; dcfg->col_timeout = atoi(p1); @@ -1515,6 +1579,9 @@ static const char *cmd_collection_timeout(cmd_parms *cmd, void *_dcfg, static const char *cmd_debug_log_level(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; dcfg->debuglog_level = atoi(p1); @@ -1526,6 +1593,8 @@ static const char *cmd_debug_log_level(cmd_parms *cmd, void *_dcfg, static const char *cmd_default_action(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; extern msc_engine *modsecurity; char *my_error_msg = NULL; @@ -1602,8 +1671,8 @@ static const char *cmd_default_action(cmd_parms *cmd, void *_dcfg, static const char *cmd_disable_backend_compression(cmd_parms *cmd, void *_dcfg, int flag) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; dcfg->disable_backend_compression = flag; return NULL; } @@ -1611,6 +1680,8 @@ static const char *cmd_disable_backend_compression(cmd_parms *cmd, void *_dcfg, static const char *cmd_guardian_log(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(p1 != NULL); extern char *guardianlog_name; extern apr_file_t *guardianlog_fd; extern char *guardianlog_condition; @@ -1671,8 +1742,8 @@ static const char *cmd_guardian_log(cmd_parms *cmd, void *_dcfg, */ static const char *cmd_stream_inbody_inspection(cmd_parms *cmd, void *_dcfg, int flag) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; dcfg->stream_inbody_inspection = flag; return NULL; } @@ -1690,8 +1761,8 @@ static const char *cmd_stream_inbody_inspection(cmd_parms *cmd, void *_dcfg, int */ static const char *cmd_stream_outbody_inspection(cmd_parms *cmd, void *_dcfg, int flag) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; dcfg->stream_outbody_inspection = flag; return NULL; } @@ -1708,11 +1779,12 @@ static const char *cmd_stream_outbody_inspection(cmd_parms *cmd, void *_dcfg, in static const char *cmd_rule_perf_time(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRulePerfTime: %s", p1); @@ -1727,6 +1799,10 @@ char *parser_conn_limits_operator(apr_pool_t *mp, const char *p2, TreeRoot **whitelist, TreeRoot **suspicious_list, const char *filename) { + assert(p2 != NULL); + assert(whitelist != NULL); + assert(suspicious_list != NULL); + assert(filename != NULL); int res = 0; char *config_orig_path; char *param = strchr(p2, ' '); @@ -1801,11 +1877,12 @@ char *parser_conn_limits_operator(apr_pool_t *mp, const char *p2, static const char *cmd_conn_read_state_limit(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX) || (limit == LONG_MIN) || (limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for " \ @@ -1829,6 +1906,7 @@ static const char *cmd_conn_read_state_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_read_state_limit(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool, "SecReadStateLimit is depricated, use SecConnReadStateLimit " \ "instead."); @@ -1850,11 +1928,12 @@ static const char *cmd_read_state_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_conn_write_state_limit(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX) || (limit == LONG_MIN) || (limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for " \ @@ -1877,6 +1956,7 @@ static const char *cmd_conn_write_state_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_write_state_limit(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool, "SecWriteStateLimit is depricated, use SecConnWriteStateLimit " \ "instead."); @@ -1889,11 +1969,12 @@ static const char *cmd_write_state_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_body_inmemory_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyInMemoryLimit: %s", p1); @@ -1907,11 +1988,12 @@ static const char *cmd_request_body_inmemory_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_body_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyLimit: %s", p1); @@ -1925,11 +2007,12 @@ static const char *cmd_request_body_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_body_no_files_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyNoFilesLimit: %s", p1); @@ -1943,11 +2026,12 @@ static const char *cmd_request_body_no_files_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_body_json_depth_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyJsonDepthLimit: %s", p1); @@ -1961,11 +2045,12 @@ static const char *cmd_request_body_json_depth_limit(cmd_parms *cmd, void *_dcfg static const char *cmd_arguments_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; - if (dcfg == NULL) return NULL; - limit = strtol(p1, NULL, 10); if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecArgumentsLimit: %s", p1); @@ -1979,9 +2064,11 @@ static const char *cmd_arguments_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (strcasecmp(p1, "on") == 0) dcfg->reqbody_access = 1; else if (strcasecmp(p1, "off") == 0) dcfg->reqbody_access = 0; @@ -2004,9 +2091,11 @@ static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_intercept_on_error(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (strcasecmp(p1, "on") == 0) dcfg->reqintercept_oe = 1; else if (strcasecmp(p1, "off") == 0) dcfg->reqintercept_oe = 0; @@ -2020,11 +2109,10 @@ static const char *cmd_request_intercept_on_error(cmd_parms *cmd, void *_dcfg, static const char *cmd_request_encoding(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + /* ENH Validate encoding */ - dcfg->request_encoding = p1; return NULL; @@ -2033,9 +2121,11 @@ static const char *cmd_request_encoding(cmd_parms *cmd, void *_dcfg, static const char *cmd_response_body_access(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (strcasecmp(p1, "on") == 0) dcfg->resbody_access = 1; else if (strcasecmp(p1, "off") == 0) dcfg->resbody_access = 0; @@ -2048,6 +2138,9 @@ static const char *cmd_response_body_access(cmd_parms *cmd, void *_dcfg, static const char *cmd_response_body_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -2068,9 +2161,11 @@ static const char *cmd_response_body_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_response_body_limit_action(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (dcfg->is_enabled == MODSEC_DETECTION_ONLY) { dcfg->of_limit_action = RESPONSE_BODY_LIMIT_ACTION_PARTIAL; return NULL; @@ -2098,9 +2193,11 @@ static const char *cmd_response_body_limit_action(cmd_parms *cmd, void *_dcfg, static const char *cmd_resquest_body_limit_action(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (dcfg->is_enabled == MODSEC_DETECTION_ONLY) { dcfg->if_limit_action = REQUEST_BODY_LIMIT_ACTION_PARTIAL; return NULL; @@ -2118,6 +2215,9 @@ static const char *cmd_resquest_body_limit_action(cmd_parms *cmd, void *_dcfg, static const char *cmd_response_body_mime_type(cmd_parms *cmd, void *_dcfg, const char *_p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(_p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; char *p1 = apr_pstrdup(cmd->pool, _p1); @@ -2136,9 +2236,9 @@ static const char *cmd_response_body_mime_type(cmd_parms *cmd, void *_dcfg, static const char *cmd_response_body_mime_types_clear(cmd_parms *cmd, void *_dcfg) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + dcfg->of_mime_types_cleared = 1; if ((dcfg->of_mime_types != NULL)&&(dcfg->of_mime_types != NOT_SET_P)) { @@ -2162,10 +2262,11 @@ static const char *cmd_response_body_mime_types_clear(cmd_parms *cmd, static const char *cmd_rule_update_target_by_id(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); - if (dcfg == NULL) return NULL; - + if(p1 == NULL) { return apr_psprintf(cmd->pool, "Updating target by ID with no ID"); } @@ -2199,10 +2300,11 @@ static const char *cmd_rule_update_target_by_id(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_update_target_by_tag(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); - if (dcfg == NULL) return NULL; - + if(p1 == NULL) { return apr_psprintf(cmd->pool, "Updating target by tag with no tag"); } @@ -2234,10 +2336,12 @@ static const char *cmd_rule_update_target_by_tag(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_update_target_by_msg(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); - if (dcfg == NULL) return NULL; - + if(p1 == NULL) { return apr_psprintf(cmd->pool, "Updating target by message with no message"); } @@ -2262,10 +2366,11 @@ static const char *cmd_rule(cmd_parms *cmd, void *_dcfg, static const char *cmd_sever_conn_filters_engine(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { conn_limits_filter_state = MODSEC_ENABLED; @@ -2289,10 +2394,11 @@ static const char *cmd_sever_conn_filters_engine(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_engine(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { dcfg->is_enabled = MODSEC_ENABLED; @@ -2318,8 +2424,11 @@ static const char *cmd_rule_engine(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_remote_rules_fail(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; + if (strncasecmp(p1, "warn", 4) == 0) { remote_rules_fail_action = REMOTE_RULES_WARN_ON_FAIL; @@ -2340,6 +2449,9 @@ static const char *cmd_remote_rules_fail(cmd_parms *cmd, void *_dcfg, const char static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2, const char *p3) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); char *error_msg = NULL; directory_config *dcfg = (directory_config *)_dcfg; #ifdef WITH_REMOTE_RULES @@ -2348,8 +2460,6 @@ static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1, const char *key = p1; #endif - if (dcfg == NULL) return NULL; - #ifdef WITH_REMOTE_RULES if (strncasecmp(p1, "crypto", 6) == 0) { @@ -2412,6 +2522,8 @@ static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1, static const char *cmd_status_engine(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); if (strcasecmp(p1, "on") == 0) { status_engine_state = STATUS_ENGINE_ENABLED; } @@ -2429,8 +2541,8 @@ static const char *cmd_status_engine(cmd_parms *cmd, void *_dcfg, const char *p1 static const char *cmd_rule_inheritance(cmd_parms *cmd, void *_dcfg, int flag) { + assert(_dcfg != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; dcfg->rule_inheritance = flag; return NULL; } @@ -2438,7 +2550,9 @@ static const char *cmd_rule_inheritance(cmd_parms *cmd, void *_dcfg, int flag) static const char *cmd_rule_script(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { - #if defined(WITH_LUA) + assert(cmd != NULL); + assert(p1 != NULL); +#if defined(WITH_LUA) const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); return add_rule(cmd, (directory_config *)_dcfg, RULE_TYPE_LUA, filename, p2, NULL); #else @@ -2450,8 +2564,10 @@ static const char *cmd_rule_script(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_remove_by_id(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; rule_exception* re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); if (re == NULL) { ap_log_perror(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, cmd->pool, "cmd_rule_remove_by_id: Cannot allocate memory"); @@ -2481,10 +2597,12 @@ static const char *cmd_rule_remove_by_id(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_remove_by_tag(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); - if (dcfg == NULL) return NULL; - + re->type = RULE_EXCEPTION_REMOVE_TAG; re->param = p1; re->param_data = msc_pregcomp(cmd->pool, p1, 0, NULL, NULL); @@ -2506,10 +2624,12 @@ static const char *cmd_rule_remove_by_tag(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_remove_by_msg(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); - if (dcfg == NULL) return NULL; - + re->type = RULE_EXCEPTION_REMOVE_MSG; re->param = p1; re->param_data = msc_pregcomp(cmd->pool, p1, 0, NULL, NULL); @@ -2531,6 +2651,8 @@ static const char *cmd_rule_remove_by_msg(cmd_parms *cmd, void *_dcfg, static const char *cmd_rule_update_action_by_id(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(p1 != NULL); int offset = 0, rule_id = atoi(p1); char *opt = strchr(p1,':'); char *savedptr = NULL; @@ -2553,6 +2675,8 @@ static const char *cmd_rule_update_action_by_id(cmd_parms *cmd, void *_dcfg, static const char *cmd_server_signature(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); if (cmd->server->is_virtual) { return "ModSecurity: SecServerSignature not allowed in VirtualHost"; } @@ -2562,10 +2686,11 @@ static const char *cmd_server_signature(cmd_parms *cmd, void *_dcfg, static const char *cmd_tmp_dir(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "none") == 0) dcfg->tmp_dir = NULL; else dcfg->tmp_dir = ap_server_root_relative(cmd->pool, p1); @@ -2574,10 +2699,11 @@ static const char *cmd_tmp_dir(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_upload_dir(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "none") == 0) dcfg->upload_dir = NULL; else dcfg->upload_dir = ap_server_root_relative(cmd->pool, p1); @@ -2587,10 +2713,11 @@ static const char *cmd_upload_dir(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_upload_file_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "default") == 0) { dcfg->upload_file_limit = NOT_SET; } @@ -2604,10 +2731,11 @@ static const char *cmd_upload_file_limit(cmd_parms *cmd, void *_dcfg, static const char *cmd_upload_filemode(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "default") == 0) { dcfg->upload_filemode = NOT_SET; } @@ -2626,10 +2754,11 @@ static const char *cmd_upload_filemode(cmd_parms *cmd, void *_dcfg, static const char *cmd_upload_keep_files(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { dcfg->upload_keep_files = KEEP_FILES_ON; } else @@ -2648,10 +2777,11 @@ static const char *cmd_upload_keep_files(cmd_parms *cmd, void *_dcfg, static const char *cmd_upload_save_tmp_files(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { dcfg->upload_validates_files = 1; @@ -2671,6 +2801,9 @@ static const char *cmd_upload_save_tmp_files(cmd_parms *cmd, void *_dcfg, static const char *cmd_web_app_id(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; /* ENH enforce format (letters, digits, ., _, -) */ @@ -2681,6 +2814,9 @@ static const char *cmd_web_app_id(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_sensor_id(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; /* ENH enforce format (letters, digits, ., _, -) */ @@ -2701,9 +2837,10 @@ static const char *cmd_sensor_id(cmd_parms *cmd, void *_dcfg, const char *p1) */ static const char *cmd_xml_external_entity(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { dcfg->xml_external_entity = 1; } @@ -2728,9 +2865,10 @@ static const char *cmd_xml_external_entity(cmd_parms *cmd, void *_dcfg, const ch */ static const char *cmd_hash_engine(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) { dcfg->hash_is_enabled = HASH_ENABLED; dcfg->hash_enforcement = HASH_ENABLED; @@ -2745,7 +2883,7 @@ static const char *cmd_hash_engine(cmd_parms *cmd, void *_dcfg, const char *p1) } /** -* \brief Add SecHashPram configuration option +* \brief Add SecHashParam configuration option * * \param cmd Pointer to configuration data * \param _dcfg Pointer to directory configuration @@ -2755,11 +2893,11 @@ static const char *cmd_hash_engine(cmd_parms *cmd, void *_dcfg, const char *p1) */ static const char *cmd_hash_param(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - - if (p1 == NULL) return NULL; dcfg->crypto_param_name = p1; return NULL; @@ -2777,12 +2915,13 @@ static const char *cmd_hash_param(cmd_parms *cmd, void *_dcfg, const char *p1) */ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, const char *_p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(_p1 != NULL); + assert(_p2 != NULL); directory_config *dcfg = (directory_config *)_dcfg; char *p1 = NULL; - if (dcfg == NULL) return NULL; - if (_p1 == NULL) return NULL; - if (strcasecmp(_p1, "Rand") == 0) { p1 = apr_pstrdup(cmd->pool, getkey(cmd->pool)); dcfg->crypto_key = p1; @@ -2793,16 +2932,13 @@ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, co dcfg->crypto_key_len = strlen(p1); } - if(_p2 == NULL) { - return NULL; - } else { - if (strcasecmp(_p2, "KeyOnly") == 0) - dcfg->crypto_key_add = HASH_KEYONLY; - else if (strcasecmp(_p2, "SessionID") == 0) - dcfg->crypto_key_add = HASH_SESSIONID; - else if (strcasecmp(_p2, "RemoteIP") == 0) - dcfg->crypto_key_add = HASH_REMOTEIP; - } + if (strcasecmp(_p2, "KeyOnly") == 0) + dcfg->crypto_key_add = HASH_KEYONLY; + else if (strcasecmp(_p2, "SessionID") == 0) + dcfg->crypto_key_add = HASH_SESSIONID; + else if (strcasecmp(_p2, "RemoteIP") == 0) + dcfg->crypto_key_add = HASH_REMOTEIP; + return NULL; } @@ -2820,6 +2956,10 @@ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, co static const char *cmd_hash_method_pm(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); + assert(p2 != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(hash_method)); const char *_p2 = apr_pstrdup(cmd->pool, p2); @@ -2827,8 +2967,6 @@ static const char *cmd_hash_method_pm(cmd_parms *cmd, void *_dcfg, const char *phrase = NULL; const char *next = NULL; - if (dcfg == NULL) return NULL; - p = acmp_create(0, cmd->pool); if (p == NULL) return NULL; @@ -2911,11 +3049,14 @@ static const char *cmd_hash_method_pm(cmd_parms *cmd, void *_dcfg, static const char *cmd_hash_method_rx(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); + assert(p2 != NULL); directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(hash_method)); const char *_p2 = apr_pstrdup(cmd->pool, p2); - if (dcfg == NULL) return NULL; - + if (strcasecmp(p1, "HashHref") == 0) { re->type = HASH_URL_HREF_HASH_RX; re->param = _p2; @@ -2978,11 +3119,11 @@ static const char *cmd_hash_method_rx(cmd_parms *cmd, void *_dcfg, */ static const char *cmd_httpBl_key(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - - if (p1 == NULL) return NULL; dcfg->httpBlkey = p1; return NULL; @@ -2993,6 +3134,8 @@ static const char *cmd_httpBl_key(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_pcre_match_limit(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); long val; if (cmd->server->is_virtual) { @@ -3012,6 +3155,8 @@ static const char *cmd_pcre_match_limit(cmd_parms *cmd, static const char *cmd_pcre_match_limit_recursion(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); long val; if (cmd->server->is_virtual) { @@ -3034,11 +3179,12 @@ static const char *cmd_pcre_match_limit_recursion(cmd_parms *cmd, static const char *cmd_geo_lookup_db(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); char *error_msg; directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (geo_init(dcfg, filename, &error_msg) <= 0) { return error_msg; } @@ -3060,6 +3206,8 @@ static const char *cmd_geo_lookup_db(cmd_parms *cmd, void *_dcfg, static const char *cmd_unicode_codepage(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); long val; val = atol(p1); @@ -3085,12 +3233,14 @@ static const char *cmd_unicode_codepage(cmd_parms *cmd, static const char *cmd_unicode_map(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(p1 != NULL); + assert(p2 != NULL); const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); char *error_msg; long val = 0; directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if(p2 != NULL) { val = atol(p2); if (val <= 0) { @@ -3120,11 +3270,12 @@ static const char *cmd_unicode_map(cmd_parms *cmd, void *_dcfg, static const char *cmd_gsb_lookup_db(cmd_parms *cmd, void *_dcfg, const char *p1) { + assert(cmd != NULL); + assert(p1 != NULL); const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); char *error_msg; directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - + if (gsb_db_init(dcfg, filename, &error_msg) <= 0) { return error_msg; } @@ -3137,10 +3288,11 @@ static const char *cmd_gsb_lookup_db(cmd_parms *cmd, void *_dcfg, static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { + assert(cmd != NULL); + assert(_dcfg != NULL); + assert(p1 != NULL); directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - if (strcasecmp(p1, "on") == 0) dcfg->cache_trans = MODSEC_CACHE_ENABLED; else if (strcasecmp(p1, "off") == 0) diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index 74b6eca9..48341774 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -675,6 +675,7 @@ static apr_status_t modsecurity_process_phase_response_headers(modsec_rec *msr) * */ static apr_status_t modsecurity_process_phase_response_body(modsec_rec *msr) { + assert(msr != NULL); apr_time_t time_before; apr_status_t rc = 0; @@ -706,6 +707,7 @@ static apr_status_t modsecurity_process_phase_response_body(modsec_rec *msr) { * */ static apr_status_t modsecurity_process_phase_logging(modsec_rec *msr) { + assert(msr != NULL); apr_time_t time_before, time_after; if (msr->txcfg->debuglog_level >= 4) { diff --git a/apache2/msc_json.c b/apache2/msc_json.c index 136e8ad9..0656fc25 100644 --- a/apache2/msc_json.c +++ b/apache2/msc_json.c @@ -21,6 +21,7 @@ const char *base_offset=NULL; int json_add_argument(modsec_rec *msr, const char *value, unsigned length) { assert(msr != NULL); + assert(msr->json != NULL); msc_arg *arg = (msc_arg *) NULL; /** @@ -89,6 +90,7 @@ static int yajl_map_key(void *ctx, const unsigned char *key, size_t length) { modsec_rec *msr = (modsec_rec *) ctx; assert(msr != NULL); + assert(msr->json != NULL); unsigned char *safe_key = (unsigned char *) NULL; /** @@ -168,6 +170,7 @@ static int yajl_number(void *ctx, const char *value, size_t length) static int yajl_start_array(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; assert(msr != NULL); + assert(msr->json != NULL); if (!msr->json->current_key && !msr->json->prefix) { msr->json->prefix = apr_pstrdup(msr->mp, "array"); @@ -198,6 +201,7 @@ static int yajl_start_array(void *ctx) { static int yajl_end_array(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; assert(msr != NULL); + assert(msr->json != NULL); unsigned char *separator = (unsigned char *) NULL; /** @@ -235,6 +239,7 @@ static int yajl_start_map(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; assert(msr != NULL); + assert(msr->json != NULL); /** * If we do not have a current_key, this is a top-level hash, so we do not @@ -274,6 +279,7 @@ static int yajl_end_map(void *ctx) { modsec_rec *msr = (modsec_rec *) ctx; assert(msr != NULL); + assert(msr->json != NULL); unsigned char *separator = (unsigned char *) NULL; /** @@ -365,6 +371,7 @@ int json_init(modsec_rec *msr, char **error_msg) { */ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char **error_msg) { assert(msr != NULL); + assert(msr->json != NULL); assert(error_msg != NULL); *error_msg = NULL; base_offset=buf; @@ -393,6 +400,7 @@ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char */ int json_complete(modsec_rec *msr, char **error_msg) { assert(msr != NULL); + assert(msr->json != NULL); assert(error_msg != NULL); char *json_data = (char *) NULL; @@ -419,6 +427,8 @@ int json_complete(modsec_rec *msr, char **error_msg) { * Frees the resources used for JSON parsing. */ apr_status_t json_cleanup(modsec_rec *msr) { + assert(msr != NULL); + assert(msr->json != NULL); msr_log(msr, 4, "JSON: Cleaning up JSON results"); if (msr->json->handle != NULL) { yajl_free(msr->json->handle); diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c index 92160adc..e8236d88 100644 --- a/apache2/msc_logging.c +++ b/apache2/msc_logging.c @@ -654,6 +654,7 @@ static void write_rule_json(modsec_rec *msr, const msre_rule *rule, yajl_gen g) * Produce an audit log entry in JSON format. */ void sec_audit_logger_json(modsec_rec *msr) { + assert(msr != NULL); const apr_array_header_t *arr = NULL; apr_table_entry_t *te = NULL; const apr_array_header_t *tarr_pattern = NULL; @@ -1547,6 +1548,7 @@ void sec_audit_logger_json(modsec_rec *msr) { * Produce an audit log entry in native format. */ void sec_audit_logger_native(modsec_rec *msr) { + assert(msr != NULL); const apr_array_header_t *arr = NULL; apr_table_entry_t *te = NULL; const apr_array_header_t *tarr_pattern = NULL; @@ -2235,7 +2237,7 @@ void sec_audit_logger_native(modsec_rec *msr) { sec_auditlog_write(msr, text, strlen(text)); } else { if ((rule != NULL) && (rule->actionset != NULL) && !rule->actionset->is_chained && (rule->chain_starter == NULL)) { - text = apr_psprintf(msr->mp, "%s\n\n", rule->unparsed); + text = apr_psprintf(msr->mp, "%s\n", rule->unparsed); sec_auditlog_write(msr, text, strlen(text)); } } @@ -2327,6 +2329,7 @@ void sec_audit_logger_native(modsec_rec *msr) { */ void sec_audit_logger(modsec_rec *msr) { #ifdef WITH_YAJL + assert(msr != NULL); if (msr->txcfg->auditlog_format == AUDITLOGFORMAT_JSON) { sec_audit_logger_json(msr); } else { diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index 7d56dd64..ae88c8ef 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -1317,6 +1317,7 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, * */ apr_status_t multipart_cleanup(modsec_rec *msr) { + assert(msr != NULL); int keep_files = 0; if (msr->mpd == NULL) return -1; diff --git a/apache2/msc_parsers.c b/apache2/msc_parsers.c index 894c84b9..9a84e2ad 100644 --- a/apache2/msc_parsers.c +++ b/apache2/msc_parsers.c @@ -245,6 +245,7 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength, apr_table_t *arguments, int *invalid_count) { assert(msr != NULL); + assert(invalid_count != NULL); msc_arg *arg; apr_size_t i, j; char *value = NULL; diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index ba8bdfd4..a52af731 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -25,6 +25,7 @@ void msre_engine_reqbody_processor_register(msre_engine *engine, const char *name, void *fn_init, void *fn_process, void *fn_complete) { + assert(engine != NULL); msre_reqbody_processor_metadata *metadata = (msre_reqbody_processor_metadata *)apr_pcalloc(engine->mp, sizeof(msre_reqbody_processor_metadata)); @@ -440,6 +441,7 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buffer, int buflen, char **error_msg) { assert(msr != NULL); assert(error_msg != NULL); + assert(buffer != NULL || buflen == 0); #ifndef MSC_LARGE_STREAM_INPUT char *stream_input_body = NULL; char *data = NULL; @@ -819,6 +821,7 @@ apr_status_t modsecurity_request_body_retrieve_start(modsec_rec *msr, char **err * */ apr_status_t modsecurity_request_body_retrieve_end(modsec_rec *msr) { + assert(msr != NULL); if (msr->msc_reqbody_storage == MSC_REQBODY_DISK) { if (msr->msc_reqbody_fd > 0) { close(msr->msc_reqbody_fd); diff --git a/apache2/msc_xml.c b/apache2/msc_xml.c index 808f7e90..2b668163 100644 --- a/apache2/msc_xml.c +++ b/apache2/msc_xml.c @@ -140,6 +140,8 @@ int xml_complete(modsec_rec *msr, char **error_msg) { * Frees the resources used for XML parsing. */ apr_status_t xml_cleanup(modsec_rec *msr) { + assert(msr != NULL); + assert(msr->xml != NULL); if (msr->xml->parsing_ctx != NULL) { if (msr->xml->parsing_ctx->myDoc) { xmlFreeDoc(msr->xml->parsing_ctx->myDoc); diff --git a/apache2/re.c b/apache2/re.c index 8e69f5ba..47a390e0 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -203,6 +203,7 @@ char *msre_ruleset_phase_rule_update_target_matching_exception(modsec_rec *msr, const char *p3) { assert(ruleset != NULL); + assert(phase_arr != NULL); msre_rule **rules; int i, j, mode; char *err; @@ -212,7 +213,10 @@ 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]; + assert(rule != NULL); + if (mode == 0) { /* Looking for next rule. */ + assert(rule->actionset != NULL); if (msre_ruleset_rule_matches_exception(rule, re)) { err = update_rule_target_ex(msr, ruleset, rule, p2, p3); if (err) return err; @@ -527,10 +531,12 @@ end: } int msre_ruleset_rule_matches_exception(msre_rule *rule, rule_exception *re) { + assert(rule != NULL); int match = 0; /* Only remove non-placeholder rules */ if (rule->placeholder == RULE_PH_NONE) { + assert(re != NULL); switch(re->type) { case RULE_EXCEPTION_REMOVE_ID : if ((rule->actionset != NULL)&&(rule->actionset->id != NULL)) { @@ -1468,6 +1474,7 @@ apr_status_t msre_ruleset_process_phase(msre_ruleset *ruleset, modsec_rec *msr) rules = (msre_rule **)arr->elts; for (i = 0; i < arr->nelts; i++) { msre_rule *rule = rules[i]; + assert(rule != NULL); rule->execution_time = 0; } @@ -1480,6 +1487,7 @@ apr_status_t msre_ruleset_process_phase(msre_ruleset *ruleset, modsec_rec *msr) rules = (msre_rule **)arr->elts; for (i = 0; i < arr->nelts; i++) { msre_rule *rule = rules[i]; + assert(rule != NULL); /* Ignore markers, which are never processed. */ if (rule->placeholder == RULE_PH_MARKER) continue; @@ -1498,6 +1506,8 @@ static apr_status_t msre_ruleset_process_phase_(msre_ruleset *ruleset, modsec_re #else apr_status_t msre_ruleset_process_phase(msre_ruleset *ruleset, modsec_rec *msr) { #endif + assert(ruleset != NULL); + assert(msr != NULL); apr_array_header_t *arr = NULL; msre_rule **rules; apr_status_t rc; @@ -1542,10 +1552,11 @@ static apr_status_t msre_ruleset_process_phase_(msre_ruleset *ruleset, modsec_re rules = (msre_rule **)arr->elts; for (i = 0; i < arr->nelts; i++) { msre_rule *rule = rules[i]; + assert(rule != NULL); + assert(rule->actionset != NULL); #if defined(PERFORMANCE_MEASUREMENT) apr_time_t time1 = 0; #endif - assert(rule->actionset != NULL); /* Reset the rule interception flag */ msr->rule_was_intercepted = 0; @@ -1974,6 +1985,9 @@ msre_ruleset *msre_ruleset_create(msre_engine *engine, apr_pool_t *mp) { * Adds one rule to the given phase of the ruleset. */ int msre_ruleset_rule_add(msre_ruleset *ruleset, msre_rule *rule, int phase) { + assert(ruleset != NULL); + assert(rule != NULL); + assert(rule->actionset != NULL); apr_array_header_t *arr = NULL; switch (phase) { @@ -2011,6 +2025,8 @@ int msre_ruleset_rule_add(msre_ruleset *ruleset, msre_rule *rule, int phase) { static msre_rule * msre_ruleset_fetch_phase_rule(const msre_ruleset *ruleset, const char *id, const apr_array_header_t *phase_arr, int offset) { + assert(id != NULL); + assert(phase_arr != NULL); msre_rule **rules = (msre_rule **)phase_arr->elts; int i; @@ -2067,6 +2083,7 @@ msre_rule * msre_ruleset_fetch_rule(msre_ruleset *ruleset, const char *id, int o static int msre_ruleset_phase_rule_remove_with_exception(msre_ruleset *ruleset, rule_exception *re, apr_array_header_t *phase_arr) { + assert(phase_arr != NULL); msre_rule **rules; int i, j, mode, removed_count; @@ -2084,6 +2101,7 @@ static int msre_ruleset_phase_rule_remove_with_exception(msre_ruleset *ruleset, /* Only remove non-placeholder rules */ if (rule->placeholder == RULE_PH_NONE) { + assert(re != NULL); switch(re->type) { case RULE_EXCEPTION_REMOVE_ID : if (rule->actionset->id != NULL) { @@ -2304,6 +2322,7 @@ char *msre_format_metadata(modsec_rec *msr, msre_actionset *actionset) { char * msre_rule_generate_unparsed(apr_pool_t *pool, const msre_rule *rule, const char *targets, const char *args, const char *actions) { + assert(rule != NULL); char *unparsed = NULL; const char *r_targets = targets; const char *r_args = args; @@ -2363,12 +2382,14 @@ msre_rule *msre_rule_create(msre_ruleset *ruleset, int type, const char *fn, int line, const char *targets, const char *args, const char *actions, char **error_msg) { + assert(ruleset != NULL); + assert(args != NULL); + assert(error_msg != NULL); msre_rule *rule; char *my_error_msg; const char *argsp; int rc; - if (error_msg == NULL) return NULL; *error_msg = NULL; rule = (msre_rule *)apr_pcalloc(ruleset->mp, sizeof(msre_rule)); @@ -2521,6 +2542,8 @@ static void msre_perform_disruptive_actions(modsec_rec *msr, msre_rule *rule, { assert(msr != NULL); assert(actionset != NULL); + assert(actionset->intercept_action_rec != NULL); + assert(actionset->intercept_action_rec->metadata != NULL); const apr_array_header_t *tarr; const apr_table_entry_t *telts; int i; @@ -2534,6 +2557,7 @@ static void msre_perform_disruptive_actions(modsec_rec *msr, msre_rule *rule, telts = (const apr_table_entry_t*)tarr->elts; for (i = 0; i < tarr->nelts; i++) { msre_action *action = (msre_action *)telts[i].val; + assert(action->metadata != NULL); if (action->metadata->type == ACTION_DISRUPTIVE) { if (action->metadata->execute != NULL) { action->metadata->execute(msr, mptmp, rule, action); @@ -2797,6 +2821,11 @@ static int execute_operator(msre_var *var, msre_rule *rule, modsec_rec *msr, * Executes rule against the given transaction. */ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { + assert(rule != NULL); + assert(rule->actionset != NULL); + assert(rule->targets != NULL); + assert(msr != NULL); + assert(msr->txcfg != NULL); const apr_array_header_t *arr = NULL; const apr_table_entry_t *te = NULL; msre_actionset *acting_actionset = NULL; @@ -3343,6 +3372,8 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { * */ static apr_status_t msre_rule_process_lua(msre_rule *rule, modsec_rec *msr) { + assert(rule != NULL); + assert(msr != NULL); msre_actionset *acting_actionset = NULL; char *my_error_msg = NULL; int rc; @@ -3380,6 +3411,7 @@ static apr_status_t msre_rule_process_lua(msre_rule *rule, modsec_rec *msr) { * */ static apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { + assert(msr != NULL); /* Use a fresh memory sub-pool for processing each rule */ if (msr->msc_rule_mptmp == NULL) { if (apr_pool_create(&msr->msc_rule_mptmp, msr->mp) != APR_SUCCESS) { diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 36f898dd..f307119f 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -27,6 +27,8 @@ static void msre_engine_action_register(msre_engine *engine, const char *name, unsigned int cardinality_group, fn_action_validate_t validate, fn_action_init_t init, fn_action_execute_t execute) { + assert(engine != NULL); + assert(name != NULL); msre_action_metadata *metadata = (msre_action_metadata *)apr_pcalloc(engine->mp, sizeof(msre_action_metadata)); if (metadata == NULL) return; @@ -93,6 +95,8 @@ msre_var *generate_single_var(modsec_rec *msr, msre_var *var, apr_array_header_t rvar->value = rval; rvar->value_len = rval_len; + assert(msr != NULL); + assert(msr->txcfg != NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "T (%d) %s: \"%s\"", rc, tfn->name, log_escape_nq_ex(mptmp, rvar->value, rvar->value_len)); @@ -172,6 +176,7 @@ apr_table_t *generate_multi_var(modsec_rec *msr, msre_var *var, apr_array_header */ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->txcfg != NULL); assert(var != NULL); char *data = NULL; apr_array_header_t *arr = NULL; @@ -321,6 +326,7 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t */ apr_status_t collection_original_setvar(modsec_rec *msr, const char *col_name, const msc_string *orig_var) { assert(msr != NULL); + assert(msr->txcfg != NULL); apr_table_t *table = NULL; msc_string *var = NULL; const char *var_name = NULL; @@ -379,6 +385,8 @@ apr_status_t collection_original_setvar(modsec_rec *msr, const char *col_name, c static apr_status_t msre_action_marker_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->id = action->param; return 1; } @@ -388,6 +396,8 @@ static apr_status_t msre_action_marker_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_id_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->id = action->param; return 1; } @@ -414,6 +424,8 @@ static char *msre_action_id_validate(msre_engine *engine, apr_pool_t *mp, msre_a static apr_status_t msre_action_rev_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->rev = action->param; return 1; } @@ -423,6 +435,8 @@ static apr_status_t msre_action_rev_init(msre_engine *engine, apr_pool_t *mp, ms static apr_status_t msre_action_msg_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->msg = action->param; return 1; } @@ -432,6 +446,8 @@ static apr_status_t msre_action_msg_init(msre_engine *engine, apr_pool_t *mp, ms static apr_status_t msre_action_logdata_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->logdata = action->param; return 1; } @@ -441,6 +457,8 @@ static apr_status_t msre_action_logdata_init(msre_engine *engine, apr_pool_t *mp static apr_status_t msre_action_sanitizeMatchedBytes_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); char *parse_parm = NULL; char *ac_param = NULL; char *savedptr = NULL; @@ -469,6 +487,8 @@ static apr_status_t msre_action_sanitizeMatchedBytes_init(msre_engine *engine, a static apr_status_t msre_action_accuracy_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->accuracy = atoi(action->param); return 1; } @@ -478,6 +498,8 @@ static apr_status_t msre_action_accuracy_init(msre_engine *engine, apr_pool_t *m static apr_status_t msre_action_maturity_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->maturity = atoi(action->param); return 1; } @@ -487,6 +509,8 @@ static apr_status_t msre_action_maturity_init(msre_engine *engine, apr_pool_t *m static apr_status_t msre_action_ver_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->version = action->param; return 1; } @@ -496,6 +520,8 @@ static apr_status_t msre_action_ver_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_severity_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); if (strcasecmp(action->param, "emergency") == 0) { actionset->severity = 0; } else if (strcasecmp(action->param, "alert") == 0) { @@ -523,6 +549,7 @@ static apr_status_t msre_action_severity_init(msre_engine *engine, apr_pool_t *m static apr_status_t msre_action_chain_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->is_chained = 1; return 1; } @@ -531,6 +558,7 @@ static apr_status_t msre_action_chain_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_log_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->log = 1; return 1; } @@ -539,6 +567,7 @@ static apr_status_t msre_action_log_init(msre_engine *engine, apr_pool_t *mp, ms static apr_status_t msre_action_nolog_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->log = 0; actionset->auditlog = 0; return 1; @@ -548,6 +577,7 @@ static apr_status_t msre_action_nolog_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_auditlog_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->auditlog = 1; return 1; } @@ -556,6 +586,7 @@ static apr_status_t msre_action_auditlog_init(msre_engine *engine, apr_pool_t *m static apr_status_t msre_action_noauditlog_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->auditlog = 0; return 1; } @@ -564,6 +595,7 @@ static apr_status_t msre_action_noauditlog_init(msre_engine *engine, apr_pool_t static apr_status_t msre_action_block_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); /* Right now we just set a flag and inherit the real disruptive action */ actionset->block = 1; return 1; @@ -573,6 +605,7 @@ static apr_status_t msre_action_block_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_deny_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); actionset->intercept_action = ACTION_DENY; actionset->intercept_action_rec = action; return 1; @@ -587,6 +620,8 @@ static char *msre_action_status_validate(msre_engine *engine, apr_pool_t *mp, ms static apr_status_t msre_action_status_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_status = atoi(action->param); return 1; } @@ -595,6 +630,8 @@ static apr_status_t msre_action_status_init(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_drop_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_DROP; actionset->intercept_action_rec = action; return 1; @@ -609,6 +646,8 @@ static char *msre_action_pause_validate(msre_engine *engine, apr_pool_t *mp, msr static apr_status_t msre_action_pause_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_PAUSE; actionset->intercept_pause = action->param; return 1; @@ -624,6 +663,8 @@ static char *msre_action_redirect_validate(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_redirect_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_REDIRECT; actionset->intercept_uri = action->param; actionset->intercept_action_rec = action; @@ -634,6 +675,8 @@ static apr_status_t msre_action_redirect_execute(modsec_rec *msr, apr_pool_t *mp msre_rule *rule, msre_action *action) { assert(msr != NULL); + assert(rule != NULL); + assert(rule->actionset != NULL); assert(action != NULL); msc_string *var = NULL; @@ -658,6 +701,8 @@ static char *msre_action_proxy_validate(msre_engine *engine, apr_pool_t *mp, msr static apr_status_t msre_action_proxy_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_PROXY; actionset->intercept_uri = action->param; actionset->intercept_action_rec = action; @@ -668,6 +713,7 @@ static apr_status_t msre_action_proxy_execute(modsec_rec *msr, apr_pool_t *mptmp msre_rule *rule, msre_action *action) { assert(msr != NULL); + assert(rule != NULL); assert(action != NULL); msc_string *var = NULL; @@ -692,6 +738,8 @@ static apr_status_t msre_action_proxy_execute(modsec_rec *msr, apr_pool_t *mptmp static apr_status_t msre_action_pass_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_NONE; actionset->intercept_action_rec = action; return 1; @@ -707,6 +755,8 @@ static char *msre_action_skip_validate(msre_engine *engine, apr_pool_t *mp, msre static apr_status_t msre_action_skip_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->skip_count = atoi(action->param); if (actionset->skip_count <= 0) actionset->skip_count = 1; return 1; @@ -722,6 +772,8 @@ static char *msre_action_skipAfter_validate(msre_engine *engine, apr_pool_t *mp, static apr_status_t msre_action_skipAfter_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->skip_after = action->param; return 1; } @@ -731,6 +783,8 @@ static apr_status_t msre_action_skipAfter_init(msre_engine *engine, apr_pool_t * static apr_status_t msre_action_allow_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); actionset->intercept_action = ACTION_ALLOW; actionset->intercept_action_rec = action; @@ -747,6 +801,7 @@ static apr_status_t msre_action_allow_init(msre_engine *engine, apr_pool_t *mp, } static char *msre_action_allow_validate(msre_engine *engine, apr_pool_t *mp, msre_action *action) { + assert(action != NULL); if (action->param != NULL) { if (strcasecmp(action->param, "phase") == 0) { return NULL; @@ -771,6 +826,8 @@ static char *msre_action_phase_validate(msre_engine *engine, apr_pool_t *mp, msr static apr_status_t msre_action_phase_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(actionset != NULL); + assert(action != NULL); if(strcasecmp(action->param,"request") == 0) actionset->phase = 2; else if(strcasecmp(action->param,"response") == 0) @@ -786,6 +843,7 @@ static apr_status_t msre_action_phase_init(msre_engine *engine, apr_pool_t *mp, /* t */ static char *msre_action_t_validate(msre_engine *engine, apr_pool_t *mp, msre_action *action) { + assert(action != NULL); msre_tfn_metadata *metadata = NULL; metadata = msre_engine_tfn_resolve(engine, action->param); if (metadata == NULL) return apr_psprintf(mp, "Invalid transformation function: %s", @@ -797,6 +855,7 @@ static char *msre_action_t_validate(msre_engine *engine, apr_pool_t *mp, msre_ac static apr_status_t msre_action_t_init(msre_engine *engine, apr_pool_t *mp, msre_actionset *actionset, msre_action *action) { + assert(action != NULL); msre_tfn_metadata *metadata = (msre_tfn_metadata *)action->param_data; action->param_data = metadata; return 1; @@ -804,6 +863,7 @@ static apr_status_t msre_action_t_init(msre_engine *engine, apr_pool_t *mp, msre /* ctl */ static char *msre_action_ctl_validate(msre_engine *engine, apr_pool_t *mp, msre_action *action) { + assert(action != NULL); char *name = NULL; char *value = NULL; @@ -1332,6 +1392,7 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, /* xmlns */ static char *msre_action_xmlns_validate(msre_engine *engine, apr_pool_t *mp, msre_action *action) { + assert(action != NULL); char *name = NULL; char *value = NULL; @@ -1392,12 +1453,14 @@ static apr_status_t msre_action_sanitizeMatched_execute(modsec_rec *msr, apr_poo const apr_table_entry_t *telts; int i, type = 0; msc_string *mvar = msr->matched_var; + assert(mvar != NULL); if (mvar->name_len == 0) return 0; /* IMP1 We need to extract the variable name properly here, * taking into account it may have been escaped. */ + assert(mvar->name != NULL); if ((mvar->name_len > 5) && (strncmp(mvar->name, "ARGS:", 5) == 0)) { sargname = apr_pstrdup(msr->mp, mvar->name + 5); type = SANITISE_ARG; @@ -1432,10 +1495,13 @@ static apr_status_t msre_action_sanitizeMatched_execute(modsec_rec *msr, apr_poo switch(type) { case SANITISE_ARG : + assert(msr->arguments_to_sanitize != NULL); tarr = apr_table_elts(msr->arguments); + assert(tarr != NULL); telts = (const apr_table_entry_t*)tarr->elts; for (i = 0; i < tarr->nelts; i++) { msc_arg *arg = (msc_arg *)telts[i].val; + assert(arg != NULL); if (strcasecmp(sargname, arg->name) == 0) { apr_table_addn(msr->arguments_to_sanitize, arg->name, (void *)arg); } @@ -1443,10 +1509,12 @@ static apr_status_t msre_action_sanitizeMatched_execute(modsec_rec *msr, apr_poo break; case SANITISE_REQUEST_HEADER : + assert(msr->request_headers_to_sanitize != NULL); apr_table_set(msr->request_headers_to_sanitize, sargname, "1"); break; case SANITISE_RESPONSE_HEADER : + assert(msr->response_headers_to_sanitize != NULL); apr_table_set(msr->response_headers_to_sanitize, sargname, "1"); break; @@ -1463,7 +1531,9 @@ static apr_status_t msre_action_sanitizeRequestHeader_execute(modsec_rec *msr, a msre_rule *rule, msre_action *action) { assert(msr != NULL); + assert(msr->request_headers_to_sanitize != NULL); assert(action != NULL); + assert(action->param != NULL); apr_table_set(msr->request_headers_to_sanitize, action->param, "1"); return 1; } @@ -1473,7 +1543,9 @@ static apr_status_t msre_action_sanitizeResponseHeader_execute(modsec_rec *msr, msre_rule *rule, msre_action *action) { assert(msr != NULL); + assert(msr->response_headers_to_sanitize != NULL); assert(action != NULL); + assert(action->param != NULL); apr_table_set(msr->response_headers_to_sanitize, action->param, "1"); return 1; } @@ -1484,6 +1556,7 @@ static apr_status_t msre_action_setenv_execute(modsec_rec *msr, apr_pool_t *mptm { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); char *data = apr_pstrdup(mptmp, action->param); char *env_name = NULL, *env_value = NULL; char *s = NULL; @@ -1501,6 +1574,7 @@ static apr_status_t msre_action_setenv_execute(modsec_rec *msr, apr_pool_t *mptm *s = '\0'; } + assert(msr->txcfg != NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "Setting env variable: %s=%s", env_name, env_value); } @@ -1519,6 +1593,7 @@ static apr_status_t msre_action_setenv_execute(modsec_rec *msr, apr_pool_t *mptm /* Execute the requested action. */ if (env_name != NULL && env_name[0] == '!') { /* Delete */ + assert(msr->r != NULL); apr_table_unset(msr->r->subprocess_env, env_name + 1); if (msr->txcfg->debuglog_level >= 9) { @@ -1539,6 +1614,7 @@ static apr_status_t msre_action_setenv_execute(modsec_rec *msr, apr_pool_t *mptm expand_macros(msr, val, rule, mptmp); /* To be safe, we escape NULs as it goes in subprocess_env. */ + assert(msr->mp != NULL); val_value = log_escape_nul(msr->mp, (const unsigned char *)val->value, val->value_len); apr_table_set(msr->r->subprocess_env, env_name, val_value); @@ -1749,6 +1825,7 @@ static apr_status_t msre_action_setvar_parse(modsec_rec *msr, apr_pool_t *mptmp, { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); char *data = apr_pstrdup(mptmp, action->param); char *var_name = NULL, *var_value = NULL; char *s = NULL; @@ -1776,6 +1853,7 @@ static apr_status_t msre_action_expirevar_execute(modsec_rec *msr, apr_pool_t *m { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); char *data = apr_pstrdup(mptmp, action->param); char *col_name = NULL, *var_name = NULL, *var_value = NULL; char *s = NULL; @@ -1875,6 +1953,7 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); char *data = apr_pstrdup(mptmp, action->param); char *col_name = NULL, *var_name = NULL, *var_value = NULL; char *s = NULL; @@ -2010,6 +2089,8 @@ static apr_status_t init_collection(modsec_rec *msr, const char *real_col_name, const char *col_name, const char *col_key, unsigned int col_key_len) { assert(msr != NULL); + assert(msr->collections != NULL); + assert(msr->txcfg != NULL); assert(real_col_name != NULL); apr_table_t *table = NULL; msc_string *var = NULL; @@ -2146,6 +2227,7 @@ static apr_status_t msre_action_initcol_execute(modsec_rec *msr, apr_pool_t *mpt { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); char *data = apr_pstrdup(msr->mp, action->param); char *col_name = NULL, *col_key = NULL; unsigned int col_key_len; @@ -2179,6 +2261,7 @@ static apr_status_t msre_action_setsid_execute(modsec_rec *msr, apr_pool_t *mptm { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); msc_string *var = NULL; char *real_col_name = NULL, *col_key = NULL; unsigned int col_key_len; @@ -2205,6 +2288,7 @@ static apr_status_t msre_action_setuid_execute(modsec_rec *msr, apr_pool_t *mptm { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); msc_string *var = NULL; char *real_col_name = NULL, *col_key = NULL; unsigned int col_key_len; @@ -2231,6 +2315,7 @@ static apr_status_t msre_action_setrsc_execute(modsec_rec *msr, apr_pool_t *mptm { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); msc_string *var = NULL; char *real_col_name = NULL, *col_key = NULL; unsigned int col_key_len; @@ -2252,6 +2337,8 @@ static apr_status_t msre_action_setrsc_execute(modsec_rec *msr, apr_pool_t *mptm /* exec */ static char *msre_action_exec_validate(msre_engine *engine, apr_pool_t *mp, msre_action *action) { + assert(action != NULL); + assert(action->param != NULL); #if defined(WITH_LUA) char *filename = (char *)action->param; @@ -2311,6 +2398,7 @@ static apr_status_t msre_action_prepend_execute(modsec_rec *msr, apr_pool_t *mpt { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); msc_string *var = NULL; /* Expand any macros in the text */ @@ -2333,6 +2421,7 @@ static apr_status_t msre_action_append_execute(modsec_rec *msr, apr_pool_t *mptm { assert(msr != NULL); assert(action != NULL); + assert(action->param != NULL); msc_string *var = NULL; /* Expand any macros in the text */ diff --git a/apache2/re_operators.c b/apache2/re_operators.c index 1d812263..612bff91 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -44,6 +44,8 @@ void msre_engine_op_register(msre_engine *engine, const char *name, fn_op_param_init_t fn1, fn_op_execute_t fn2) { + assert(engine != NULL); + assert(name != NULL); msre_op_metadata *metadata = (msre_op_metadata *)apr_pcalloc(engine->mp, sizeof(msre_op_metadata)); if (metadata == NULL) return; @@ -58,6 +60,7 @@ void msre_engine_op_register(msre_engine *engine, const char *name, * */ msre_op_metadata *msre_engine_op_resolve(msre_engine *engine, const char *name) { + assert(engine != NULL); return (msre_op_metadata *)apr_table_get(engine->operators, name); } @@ -101,13 +104,12 @@ static int msre_op_nomatch_execute(modsec_rec *msr, msre_rule *rule, * \retval 0 On Fail */ static int msre_op_ipmatch_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(error_msg != NULL); char *param = NULL; int res = 0; - if (error_msg == NULL) - return -1; - else - *error_msg = NULL; + *error_msg = NULL; param = apr_pstrdup(rule->ruleset->mp, rule->op_param); @@ -173,6 +175,8 @@ static int msre_op_ipmatch_execute(modsec_rec *msr, msre_rule *rule, msre_var *v * \retval 0 On Fail */ static int msre_op_ipmatchFromFile_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(error_msg != NULL); const char *rootpath = NULL; const char *filepath = NULL; const char *ipfile_path = NULL; @@ -300,6 +304,8 @@ static int msre_op_ipmatchFromFile_execute(modsec_rec *msr, msre_rule *rule, /* rsub */ static char *param_remove_escape(msre_rule *rule, char *str, int len) { + assert(rule != NULL); + assert(str != NULL); char *parm = apr_pcalloc(rule->ruleset->mp, len); char *ret = parm; @@ -332,6 +338,9 @@ static char *param_remove_escape(msre_rule *rule, char *str, int len) { */ #if !defined(MSC_TEST) static int msre_op_rsub_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 0 ap_regex_t *regex; #else @@ -349,7 +358,6 @@ static int msre_op_rsub_param_init(msre_rule *rule, char **error_msg) { int ignore_case = 0; unsigned short int op_len = 0; - if (error_msg == NULL) return -1; *error_msg = NULL; line = rule->op_param; @@ -686,6 +694,8 @@ nextround: * \retval 0 On fail */ static int msre_op_validateHash_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; msc_regex_t *regex; @@ -961,6 +971,8 @@ static int msre_op_validateHash_execute(modsec_rec *msr, msre_rule *rule, msre_v /* rx */ static int msre_op_rx_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; msc_regex_t *regex; @@ -1282,6 +1294,9 @@ static int msre_op_rx_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c /* pm */ static int msre_op_pm_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); ACMP *p; const char *phrase; const char *next; @@ -1320,6 +1335,9 @@ static int msre_op_pm_param_init(msre_rule *rule, char **error_msg) { /* pmFromFile */ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); char errstr[1024]; char buf[HUGE_STRING_LEN + 1]; char *fn = NULL; @@ -1567,7 +1585,7 @@ static int msre_op_pm_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c * \retval url On Success */ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int len) { - + assert(domain != NULL); char *pos = NULL, *data = NULL; char *url = NULL; int match = 0; @@ -1605,7 +1623,7 @@ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int l * \retval reduced On Success */ static const char *gsb_reduce_char(apr_pool_t *pool, const char *domain) { - + assert(domain != NULL); char *ptr = apr_pstrdup(pool, domain); char *data = NULL; char *reduced = NULL; @@ -1712,6 +1730,8 @@ static int verify_gsb(gsb_db *gsb, modsec_rec *msr, const char *match, unsigned * \retval 0 On Fail */ static int msre_op_gsbLookup_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; int options = 0; @@ -2095,6 +2115,10 @@ static int msre_op_gsbLookup_execute(modsec_rec *msr, msre_rule *rule, msre_var /* within */ static int msre_op_within_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) { + assert(msr != NULL); + assert(rule != NULL); + assert(var != NULL); + assert(error_msg != NULL); msc_string *str = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string)); const char *match = NULL; const char *target; @@ -2255,7 +2279,6 @@ static int msre_op_detectSQLi_execute(modsec_rec *msr, msre_rule *rule, msre_var assert(rule->actionset != NULL); assert(rule->actionset->actions != NULL); assert(var != NULL); - assert(var != NULL); assert(error_msg != NULL); char fingerprint[8]; int issqli; @@ -2292,6 +2315,7 @@ static int msre_op_detectXSS_execute(modsec_rec *msr, msre_rule *rule, msre_var assert(rule != NULL); assert(rule->actionset != NULL); assert(rule->actionset->actions != NULL); + assert(var != NULL); assert(error_msg != NULL); int capture; int is_xss; @@ -2554,6 +2578,8 @@ static int msre_op_beginsWith_execute(modsec_rec *msr, msre_rule *rule, msre_var static int msre_op_endsWith_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) { assert(msr != NULL); assert(rule != NULL); + assert(var != NULL); + assert(error_msg != NULL); const char *match = NULL; const char *target; unsigned int match_length; @@ -2619,12 +2645,14 @@ static int msre_op_endsWith_execute(modsec_rec *msr, msre_rule *rule, msre_var * /* strmatch */ static int msre_op_strmatch_param_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); const apr_strmatch_pattern *compiled_pattern; char *processed = NULL; const char *pattern = rule->op_param; unsigned short int op_len; - if (error_msg == NULL) return -1; *error_msg = NULL; op_len = strlen(pattern); @@ -2854,6 +2882,7 @@ static int msre_op_validateSchema_execute(modsec_rec *msr, msre_rule *rule, msre * Luhn Mod-10 Method (ISO 2894/ANSI 4.13) */ static int luhn_verify(const char *ccnumber, int len) { + assert(ccnumber != NULL); int sum[2] = { 0, 0 }; int odd = 0; int digits = 0; @@ -2887,6 +2916,9 @@ static int luhn_verify(const char *ccnumber, int len) { } static int msre_op_verifyCC_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; int options = 0; @@ -2916,6 +2948,7 @@ static int msre_op_verifyCC_init(msre_rule *rule, char **error_msg) { static int msre_op_verifyCC_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) { assert(msr != NULL); assert(rule != NULL); + assert(rule->actionset != NULL); assert(var != NULL); assert(error_msg != NULL); msc_regex_t *regex = (msc_regex_t *)rule->op_param_data; @@ -3119,7 +3152,7 @@ static int msre_op_verifyCC_execute(modsec_rec *msr, msre_rule *rule, msre_var * * \retval 1 On Valid CPF */ static int cpf_verify(const char *cpfnumber, int len) { - + assert(cpfnumber != NULL); int factor, part_1, part_2, var_len = len; unsigned int sum = 0, i = 0, cpf_len = 11, c; int cpf[11]; @@ -3207,12 +3240,14 @@ static int cpf_verify(const char *cpfnumber, int len) { * \retval 1 On Success */ static int msre_op_verifyCPF_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; int options = 0; msc_regex_t *regex; - if (error_msg == NULL) return -1; *error_msg = NULL; #ifdef WITH_PCRE2 @@ -3527,6 +3562,9 @@ invalid: * \retval 1 On Success */ static int msre_op_verifySSN_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); const char *errptr = NULL; int erroffset; int options = 0; @@ -4085,6 +4123,9 @@ static int msre_op_rbl_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, /* fuzzyHash */ static int msre_op_fuzzy_hash_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); #ifdef WITH_SSDEEP struct fuzzy_hash_param_data *param_data; struct fuzzy_hash_chunk *chunk, *t; @@ -4105,11 +4146,6 @@ static int msre_op_fuzzy_hash_init(msre_rule *rule, char **error_msg) threshold_str = data; #endif - if (error_msg == NULL) - { - return -1; - } - *error_msg = NULL; #ifdef WITH_SSDEEP @@ -4241,9 +4277,11 @@ static int msre_op_fuzzy_hash_execute(modsec_rec *msr, msre_rule *rule, /* inspectFile */ static int msre_op_inspectFile_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); char *filename = (char *)rule->op_param; - if (error_msg == NULL) return -1; *error_msg = NULL; if ((filename == NULL)||(is_empty_string(filename))) { @@ -4346,6 +4384,9 @@ static int msre_op_inspectFile_execute(modsec_rec *msr, msre_rule *rule, msre_va /* validateByteRange */ static int msre_op_validateByteRange_init(msre_rule *rule, char **error_msg) { + assert(rule != NULL); + assert(rule->ruleset != NULL); + assert(error_msg != NULL); char *p = NULL, *saveptr = NULL; char *table = NULL, *data = NULL; @@ -4671,6 +4712,7 @@ static int msre_op_eq_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, { assert(msr != NULL); assert(rule != NULL); + assert(var != NULL); assert(error_msg != NULL); msc_string str; int left, right; diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 5aa7589a..a37124d5 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -30,6 +30,9 @@ static int var_simple_generate_ex(msre_var *var, apr_table_t *vartab, apr_pool_t *mptmp, const char *value, int value_len) { + assert(var != NULL); + assert(vartab != NULL); + assert(mptmp != NULL); msre_var *rvar = NULL; if (value == NULL) return 0; @@ -57,6 +60,9 @@ static int var_simple_generate(msre_var *var, apr_table_t *vartab, apr_pool_t *m * care of the case when the parameter is a regular expression. */ static char *var_generic_list_validate(msre_ruleset *ruleset, msre_var *var) { + assert(ruleset != NULL); + assert(var != NULL); + /* It's OK if there's no parameter. */ if (var->param == NULL) return NULL; @@ -112,6 +118,7 @@ static int var_args_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Figure out if we want to include this argument. */ @@ -162,6 +169,7 @@ static int var_args_combined_size_generate(modsec_rec *msr, msre_var *var, msre_ te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); combined_size += arg->name_len; combined_size += arg->value_len; } @@ -191,6 +199,7 @@ static int var_args_names_generate(modsec_rec *msr, msre_var *var, msre_rule *ru te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Figure out if we want to include this variable. */ @@ -239,6 +248,7 @@ static int var_args_get_generate(modsec_rec *msr, msre_var *var, msre_rule *rule te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Only QUERY_STRING arguments */ @@ -290,6 +300,7 @@ static int var_args_get_names_generate(modsec_rec *msr, msre_var *var, msre_rule te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Only QUERY_STRING arguments */ @@ -341,6 +352,7 @@ static int var_args_post_generate(modsec_rec *msr, msre_var *var, msre_rule *rul te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Only BODY arguments */ @@ -392,6 +404,7 @@ static int var_args_post_names_generate(modsec_rec *msr, msre_var *var, msre_rul te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_arg *arg = (msc_arg *)te[i].val; + assert(arg != NULL); int match = 0; /* Only BODY arguments */ @@ -476,6 +489,7 @@ static int var_rule_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, static char *var_env_validate(msre_ruleset *ruleset, msre_var *var) { assert(ruleset != NULL); + assert(var != NULL); if (var->param == NULL) { return apr_psprintf(ruleset->mp, "Parameter required for ENV."); } @@ -491,6 +505,7 @@ static int var_env_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); assert(var != NULL); char *value = get_env_var(msr->r, (char *)var->param); if (value != NULL) { @@ -505,6 +520,7 @@ static int var_request_uri_raw_generate(modsec_rec *msr, msre_var *var, msre_rul apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); return var_simple_generate(var, vartab, mptmp, msr->r->unparsed_uri); } @@ -551,6 +567,8 @@ static int var_reqbody_processor_generate(modsec_rec *msr, msre_var *var, msre_r apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(var != NULL); + assert(vartab != NULL); msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); if (!rvar) { msr_log(msr, 1, "REQBODY_PROCESSOR: Memory allocation error"); @@ -575,7 +593,6 @@ static int var_sdbm_delete_error_generate(modsec_rec *msr, msre_var *var, msre_r apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); - assert(msr->r != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -602,6 +619,9 @@ static int var_reqbody_processor_error_generate(modsec_rec *msr, msre_var *var, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(var != NULL); + assert(vartab != NULL); + assert(mptmp != NULL); msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); if (!rvar) { msr_log(msr, 1, "REQBODY_ERROR: Memory allocation error"); @@ -622,7 +642,6 @@ static int var_reqbody_processor_error_msg_generate(modsec_rec *msr, msre_var *v { assert(msr != NULL); assert(var != NULL); - assert(rule != NULL); assert(vartab != NULL); assert(mptmp != NULL); msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); @@ -647,6 +666,8 @@ static int var_reqbody_processor_error_msg_generate(modsec_rec *msr, msre_var *v /* XML */ static char *var_xml_validate(msre_ruleset *ruleset, msre_var *var) { + assert(var != NULL); + /* It's OK if there's no parameter. */ if (var->param == NULL) return NULL; @@ -835,6 +856,8 @@ static int var_remote_addr_generate(modsec_rec *msr, msre_var *var, msre_rule *r #if !defined(MSC_TEST) #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 3 if (ap_find_linked_module("mod_remoteip.c") != NULL) { + assert(msr->r != NULL); + assert(msr->r->useragent_ip != NULL); if(msr->r->useragent_ip != NULL) msr->remote_addr = apr_pstrdup(msr->mp, msr->r->useragent_ip); return var_simple_generate(var, vartab, mptmp, msr->remote_addr); } @@ -850,6 +873,7 @@ static int var_remote_host_generate(modsec_rec *msr, msre_var *var, msre_rule *r apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); const char *value1 = ap_get_remote_host(msr->r->connection, msr->r->per_dir_config, REMOTE_NAME, NULL); return var_simple_generate(var, vartab, mptmp, value1); @@ -888,9 +912,11 @@ static int var_tx_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, int i, count = 0; arr = apr_table_elts(msr->tx_vars); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -944,9 +970,11 @@ static int var_geo_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, int i, count = 0; arr = apr_table_elts(msr->geo_vars); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -983,6 +1011,7 @@ static int var_geo_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, static int var_highest_severity_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { + assert(msr != NULL); return var_simple_generate(var, vartab, mptmp, apr_psprintf(mptmp, "%d", msr->highest_severity)); } @@ -993,6 +1022,7 @@ static int var_ip_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->collections != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -1005,9 +1035,11 @@ static int var_ip_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, if (target_col == NULL) return 0; arr = apr_table_elts(target_col); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -1073,6 +1105,7 @@ static int var_session_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->collections != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -1088,6 +1121,7 @@ static int var_session_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -1129,6 +1163,7 @@ static int var_user_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->collections != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -1144,6 +1179,7 @@ static int var_user_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -1189,6 +1225,7 @@ static int var_global_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->collections != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -1204,6 +1241,7 @@ static int var_global_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -1241,6 +1279,7 @@ static int var_resource_generate(modsec_rec *msr, msre_var *var, msre_rule *rule apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->collections != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -1256,6 +1295,7 @@ static int var_resource_generate(modsec_rec *msr, msre_var *var, msre_rule *rule te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); int match; /* Figure out if we want to include this variable. */ @@ -1304,6 +1344,7 @@ static int var_files_tmp_contents_generate(modsec_rec *msr, msre_var *var, int i, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for (i = 0; i < msr->mpd->parts->nelts; i++) @@ -1405,9 +1446,11 @@ static int var_files_tmpnames_generate(modsec_rec *msr, msre_var *var, msre_rule int i, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); if ((parts[i]->type == MULTIPART_FILE)&&(parts[i]->tmp_file_name != NULL)) { int match = 0; @@ -1454,9 +1497,11 @@ static int var_files_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, int i, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); if (parts[i]->type == MULTIPART_FILE) { int match = 0; @@ -1503,9 +1548,11 @@ static int var_files_sizes_generate(modsec_rec *msr, msre_var *var, msre_rule *r int i, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); if (parts[i]->type == MULTIPART_FILE) { int match = 0; @@ -1552,9 +1599,11 @@ static int var_files_names_generate(modsec_rec *msr, msre_var *var, msre_rule *r int i, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); if (parts[i]->type == MULTIPART_FILE) { msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); if (!rvar) { @@ -1591,6 +1640,7 @@ static int var_files_combined_size_generate(modsec_rec *msr, msre_var *var, msre if (msr->mpd != NULL) { parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); if (parts[i]->type == MULTIPART_FILE) { combined_size += parts[i]->tmp_file_size; } @@ -1622,9 +1672,11 @@ static int var_multipart_part_headers_generate(modsec_rec *msr, msre_var *var, m int i, j, count = 0; if (msr->mpd == NULL) return 0; + assert(msr->mpd->parts != NULL); parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { + assert(parts[i] != NULL); int match = 0; /* Figure out if we want to include this variable. */ @@ -1937,6 +1989,7 @@ static int var_outbound_error_generate(modsec_rec *msr, msre_var *var, msre_rule } static apr_time_t calculate_perf_combined(modsec_rec *msr) { + assert(msr != NULL); return msr->time_phase1 + msr->time_phase2 + msr->time_phase3 + msr->time_phase4 + msr->time_phase5 + msr->time_storage_write /* time_storage_read is already included in phases */ + msr->time_logging + msr->time_gc; @@ -1957,9 +2010,8 @@ char *format_all_performance_variables(modsec_rec *msr, apr_pool_t *mp) { static int generate_performance_variable(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp, apr_time_t value) { - assert(msr != NULL); assert(var != NULL); - assert( vartab!= NULL); + assert(vartab != NULL); assert(mptmp != NULL); msre_var *rvar = NULL; @@ -1977,7 +2029,6 @@ static int generate_performance_variable(modsec_rec *msr, msre_var *var, msre_ru static int var_perf_all_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { - assert(msr != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -2096,6 +2147,7 @@ static int var_perf_rules_generate(modsec_rec *msr, msre_var *var, msre_rule *ru int i, count = 0; arr = apr_table_elts(msr->perf_rules); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -2359,11 +2411,6 @@ static int var_time_mon_generate(modsec_rec *msr, msre_var *var, msre_rule *rule tm = localtime(&tc); rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); assert(msr != NULL); - assert(msr->r != NULL); - assert(var != NULL); - assert(rule != NULL); - assert(vartab != NULL); - assert(mptmp != NULL); if (!rvar) { msr_log(msr, 1, "TIME_MON: Memory allocation error"); return -1; @@ -2385,7 +2432,6 @@ static int var_time_day_generate(modsec_rec *msr, msre_var *var, msre_rule *rule apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); - assert(msr->r != NULL); assert(var != NULL); assert(vartab != NULL); assert(mptmp != NULL); @@ -2465,6 +2511,10 @@ static int var_request_basename_generate(modsec_rec *msr, msre_var *var, msre_ru static int var_full_request_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { + assert(msr != NULL); + assert(var != NULL); + assert(vartab != NULL); + assert(mptmp != NULL); const apr_array_header_t *arr = NULL; char *full_request = NULL; int full_request_length = 0; @@ -2585,6 +2635,7 @@ static int var_matched_vars_names_generate(modsec_rec *msr, msre_var *var, msre_ for (i = 0; i < arr->nelts; i++) { int match = 0; msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; @@ -2650,10 +2701,12 @@ static int var_matched_vars_generate(modsec_rec *msr, msre_var *var, msre_rule * int i, count = 0; arr = apr_table_elts(msr->matched_vars); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; msc_string *str = (msc_string *)te[i].val; + assert(str != NULL); /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; @@ -2719,6 +2772,7 @@ static int var_request_cookies_generate(modsec_rec *msr, msre_var *var, msre_rul int i, count = 0; arr = apr_table_elts(msr->request_cookies); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -2726,6 +2780,7 @@ static int var_request_cookies_generate(modsec_rec *msr, msre_var *var, msre_rul /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -2774,6 +2829,7 @@ static int var_request_cookies_names_generate(modsec_rec *msr, msre_var *var, ms int i, count = 0; arr = apr_table_elts(msr->request_cookies); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -2781,6 +2837,7 @@ static int var_request_cookies_names_generate(modsec_rec *msr, msre_var *var, ms /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -2829,6 +2886,7 @@ static int var_request_headers_generate(modsec_rec *msr, msre_var *var, msre_rul int i, count = 0; arr = apr_table_elts(msr->request_headers); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -2836,6 +2894,7 @@ static int var_request_headers_generate(modsec_rec *msr, msre_var *var, msre_rul /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -2884,6 +2943,7 @@ static int var_request_headers_names_generate(modsec_rec *msr, msre_var *var, ms int i, count = 0; arr = apr_table_elts(msr->request_headers); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -2891,6 +2951,7 @@ static int var_request_headers_names_generate(modsec_rec *msr, msre_var *var, ms /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -2931,6 +2992,7 @@ static int var_request_filename_generate(modsec_rec *msr, msre_var *var, msre_ru apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); return var_simple_generate(var, vartab, mptmp, msr->r->parsed_uri.path); } @@ -3090,6 +3152,7 @@ static int var_auth_type_generate(modsec_rec *msr, msre_var *var, msre_rule *rul apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); char *value = msr->r->ap_auth_type; return var_simple_generate(var, vartab, mptmp, value); } @@ -3100,6 +3163,8 @@ static int var_path_info_generate(modsec_rec *msr, msre_var *var, msre_rule *rul apr_table_t *vartab, apr_pool_t *mptmp) { assert(msr != NULL); + assert(msr->r != NULL); + assert(mptmp != NULL); const char *value = msr->r->path_info; return var_simple_generate(var, vartab, mptmp, value); } @@ -3162,6 +3227,7 @@ static int var_response_headers_generate(modsec_rec *msr, msre_var *var, msre_ru if (msr->response_headers == NULL) return 0; arr = apr_table_elts(msr->response_headers); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -3169,6 +3235,7 @@ static int var_response_headers_generate(modsec_rec *msr, msre_var *var, msre_ru /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -3217,6 +3284,7 @@ static int var_response_headers_names_generate(modsec_rec *msr, msre_var *var, m int i, count = 0; arr = apr_table_elts(msr->response_headers); + assert(arr != NULL); te = (apr_table_entry_t *)arr->elts; for (i = 0; i < arr->nelts; i++) { int match = 0; @@ -3224,6 +3292,7 @@ static int var_response_headers_names_generate(modsec_rec *msr, msre_var *var, m /* Figure out if we want to include this variable. */ if (var->param == NULL) match = 1; else { + assert(te[i].key != NULL); if (var->param_data != NULL) { /* Regex. */ char *my_error_msg = NULL; if (!(msc_regexec((msc_regex_t *)var->param_data, te[i].key, @@ -3350,6 +3419,7 @@ void msre_engine_variable_register(msre_engine *engine, const char *name, fn_var_validate_t validate, fn_var_generate_t generate, unsigned int is_cacheable, unsigned int availability) { + assert(engine != NULL); msre_var_metadata *metadata = (msre_var_metadata *)apr_pcalloc(engine->mp, sizeof(msre_var_metadata)); if (metadata == NULL) return; @@ -3370,6 +3440,7 @@ void msre_engine_variable_register(msre_engine *engine, const char *name, * */ void msre_engine_register_default_variables(msre_engine *engine) { + assert(engine != NULL); /* ARGS */ msre_engine_variable_register(engine, From 8dd5d5f46ba1d9588b23c7b64fd3a45bddfb5117 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 7 Aug 2024 09:42:40 +0200 Subject: [PATCH 10/20] re_operators.c: removed invalid check (done correctly on line 1067) copy_rules(): only one return code => void --- apache2/apache2_config.c | 10 ++-------- apache2/re_operators.c | 4 ---- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index cc37d8eb..895c0b42 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -293,15 +293,14 @@ static void copy_rules_phase(apr_pool_t *mp, * @retval -1 Something went wrong. * */ -static int copy_rules(apr_pool_t *mp, msre_ruleset *parent_ruleset, +static void copy_rules(apr_pool_t *mp, msre_ruleset *parent_ruleset, msre_ruleset *child_ruleset, apr_array_header_t *exceptions_arr) { assert(parent_ruleset != NULL); assert(child_ruleset != NULL); assert(exceptions_arr != NULL); - int ret = 0; - + copy_rules_phase(mp, parent_ruleset->phase_request_headers, child_ruleset->phase_request_headers, exceptions_arr); copy_rules_phase(mp, parent_ruleset->phase_request_body, @@ -312,9 +311,6 @@ static int copy_rules(apr_pool_t *mp, msre_ruleset *parent_ruleset, child_ruleset->phase_response_body, exceptions_arr); copy_rules_phase(mp, parent_ruleset->phase_logging, child_ruleset->phase_logging, exceptions_arr); - -failed: - return ret; } /** @@ -439,7 +435,6 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) /* Copy the rules from the parent context. */ merged->ruleset = msre_ruleset_create(parent->ruleset->engine, mp); - /* TODO: copy_rules return code should be taken into consideration. */ copy_rules(mp, parent->ruleset, merged->ruleset, child->rule_exceptions); } else if (parent->ruleset == NULL) { @@ -466,7 +461,6 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) /* Copy parent rules, then add child rules to it. */ merged->ruleset = msre_ruleset_create(parent->ruleset->engine, mp); - /* TODO: copy_rules return code should be taken into consideration. */ copy_rules(mp, parent->ruleset, merged->ruleset, child->rule_exceptions); apr_array_cat(merged->ruleset->phase_request_headers, diff --git a/apache2/re_operators.c b/apache2/re_operators.c index 612bff91..73b0d8fb 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -1037,10 +1037,6 @@ static int msre_op_rx_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c assert(var != NULL); assert(error_msg != NULL); msc_regex_t *regex = (msc_regex_t *)rule->op_param_data; - if (!regex) { - msr_log(msr, 1, "rx: Memory allocation error"); - return -1; - } msc_string *re_pattern = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string)); if (!re_pattern) { msr_log(msr, 1, "rx: Memory allocation error"); From 692710cab7a1bbe13ac9dd8ae3f42c02c64595b6 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 7 Aug 2024 13:45:09 +0200 Subject: [PATCH 11/20] Replaced 0 by '\0' for char --- apache2/re_operators.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apache2/re_operators.c b/apache2/re_operators.c index 30176c51..5beccc8d 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -1565,7 +1565,7 @@ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int l data = apr_palloc(pool, len + 1); if (!data) return NULL; - url[len] = 0; + url[len] = '\0'; while(( pos = strstr(url , "/./" )) != NULL) { match = 1; From 686a74173ff1a7d3fbca777194643980bd278760 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 7 Aug 2024 17:01:20 +0200 Subject: [PATCH 12/20] # Send some requests & check log format --- .github/workflows/ci.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c32ce18..d4925d44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,4 +49,14 @@ jobs: run: | sudo systemctl restart apache2.service sudo cat /var/log/apache2/error.log - + - name: Check error.log + run: | + # Send requests & check log format + # Valid request + curl -s http://127.0.01/ > /dev/null || echo $? + # Invalid request + curl -s http://127.0.01/%2e%2f > /dev/null || echo $? + # Check log format + grep -F ModSecurity < /var/log/apache2/error.log | grep -vP "^\[[^\]]+\] \[security2:[a-z]+\] \[pid [0-9]+:tid [0-9]+\] (?:\[client [0-9.:]+\] )?ModSecurity" || exit 0 + # grep -v succeeded => found some lines with invalid format + exit 1 From d704af657ca8ea10400fdb60424ef57c42719245 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 8 Aug 2024 16:16:14 +0200 Subject: [PATCH 13/20] Define _FORTIFY_SOURCE=3 & _GLIBCXX_ASSERTIONS that add glibc/libstdc++ assertions. See https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html & https://gcc.gnu.org/wiki/LibstdcxxDebugMode _GLIBCXX_ASSERTIONS is probably useless as we have pure C here, but let's define it in case some checks are included (or will be in a future version). As we handle some requests here, that may help to trap a problem. --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c75335c1..69228a6b 100644 --- a/configure.ac +++ b/configure.ac @@ -309,10 +309,10 @@ fi AC_ARG_ENABLE(assertions, AS_HELP_STRING([--enable-assertions], - [Turn on assertions checks (undefine NDEBUG)]), + [Turn on assertions checks (undefine NDEBUG, define _GLIBCXX_ASSERTIONS & _FORTIFY_SOURCE)]), [ if test "${enableval}" = "yes"; then - assertions='-UNDEBUG' + assertions='-UNDEBUG -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS' else assertions='-DNDEBUG' fi From f27c85cf472465377e9658723ddfadb44a25118d Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Tue, 13 Aug 2024 11:07:18 +0200 Subject: [PATCH 14/20] Check if the MP header contains invalid character --- apache2/msc_multipart.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index 7d56dd64..92f466e3 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -402,7 +402,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { if (msr->mpd->mpp->last_header_line != NULL) { *(char **)apr_array_push(msr->mpd->mpp->header_lines) = msr->mpd->mpp->last_header_line; msr_log(msr, 9, "Multipart: Added part header line \"%s\"", msr->mpd->mpp->last_header_line); - } + } data = msr->mpd->buf; @@ -424,6 +424,16 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { return -1; } + /* check if multipart header contains any invalid characters */ + char *ch = header_name; + while(*ch != '\0') { + if (*ch < 33 || *ch > 126) { + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (contains invalid character)."); + return -1; + } + ch++; + } + /* extract the value value */ data++; while((*data == '\t') || (*data == ' ')) data++; From e6e3417e9d642eadac2136c1ded2f36cc5cf5b92 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Tue, 13 Aug 2024 11:07:44 +0200 Subject: [PATCH 15/20] Remove unnecessary assert() --- apache2/re_variables.c | 1 - 1 file changed, 1 deletion(-) diff --git a/apache2/re_variables.c b/apache2/re_variables.c index b3ea4fac..5bf307af 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -616,7 +616,6 @@ static int var_reqbody_processor_error_msg_generate(modsec_rec *msr, msre_var *v { assert(msr != NULL); assert(var != NULL); - assert(rule != NULL); assert(vartab != NULL); assert(mptmp != NULL); msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var)); From e5bbd89399fe98280ba4dafb4b2693a8f1a660e2 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 14 Aug 2024 13:53:52 +0200 Subject: [PATCH 16/20] re-added some NULL check at run-time, with an error message on stderr --- apache2/apache2_config.c | 274 ++++++++++++++++++++++++++++++++++++++- apache2/re.c | 5 + apache2/re_operators.c | 27 ++++ 3 files changed, 305 insertions(+), 1 deletion(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 895c0b42..0c79fa3f 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -300,6 +300,13 @@ static void copy_rules(apr_pool_t *mp, msre_ruleset *parent_ruleset, assert(parent_ruleset != NULL); assert(child_ruleset != NULL); assert(exceptions_arr != NULL); + // Normally useless code, left to be safe for the moment + if (parent_ruleset == NULL || child_ruleset == NULL || exceptions_arr == NULL) { + if (parent_ruleset == NULL) ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, 0, 0, mp, "copy_rules: parent_ruleset is NULL"); + if (child_ruleset == NULL) ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, 0, mp, "copy_rules: child_ruleset is NULL"); + if (exceptions_arr == NULL) ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, 0, mp, "copy_rules: exceptions_arr is NULL"); + return; + } copy_rules_phase(mp, parent_ruleset->phase_request_headers, child_ruleset->phase_request_headers, exceptions_arr); @@ -1161,6 +1168,11 @@ static const char *cmd_action(cmd_parms *cmd, void *_dcfg, const char *p1) static const char *cmd_marker(cmd_parms *cmd, void *_dcfg, const char *p1) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_marker: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; const char *action = apr_pstrcat(dcfg->mp, SECMARKER_BASE_ACTIONS, p1, NULL); return add_marker(cmd, (directory_config *)_dcfg, SECMARKER_TARGETS, SECMARKER_ARGS, action); @@ -1172,6 +1184,11 @@ static const char *cmd_cookiev0_separator(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_cookiev0_separator: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strlen(p1) != 1) { @@ -1189,6 +1206,11 @@ static const char *cmd_argument_separator(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_argument_separator: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strlen(p1) != 1) { @@ -1383,6 +1405,11 @@ static const char *cmd_audit_log_dirmode(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_audit_log_dirmode: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "default") == 0) { @@ -1406,6 +1433,11 @@ static const char *cmd_audit_log_filemode(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_audit_log_filemode: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "default") == 0) { @@ -1491,6 +1523,11 @@ static const char *cmd_component_signature(cmd_parms *cmd, void *_dcfg, { assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_component_signature: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; /* ENH Enforce "Name/VersionX.Y.Z (comment)" format. */ @@ -1502,6 +1539,11 @@ static const char *cmd_component_signature(cmd_parms *cmd, void *_dcfg, static const char *cmd_content_injection(cmd_parms *cmd, void *_dcfg, int flag) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_content_injection: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->content_injection_enabled = flag; return NULL; @@ -1666,7 +1708,12 @@ static const char *cmd_default_action(cmd_parms *cmd, void *_dcfg, static const char *cmd_disable_backend_compression(cmd_parms *cmd, void *_dcfg, int flag) { assert(_dcfg != NULL); - directory_config *dcfg = (directory_config *)_dcfg; + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_disable_backend_compression: _dcfg is NULL"); + return NULL; + } + directory_config* dcfg = (directory_config*)_dcfg; dcfg->disable_backend_compression = flag; return NULL; } @@ -1737,6 +1784,11 @@ static const char *cmd_guardian_log(cmd_parms *cmd, void *_dcfg, static const char *cmd_stream_inbody_inspection(cmd_parms *cmd, void *_dcfg, int flag) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_stream_inbody_inspection: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->stream_inbody_inspection = flag; return NULL; @@ -1756,6 +1808,11 @@ static const char *cmd_stream_inbody_inspection(cmd_parms *cmd, void *_dcfg, int static const char *cmd_stream_outbody_inspection(cmd_parms *cmd, void *_dcfg, int flag) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_stream_outbody_inspection: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->stream_outbody_inspection = flag; return NULL; @@ -1776,6 +1833,11 @@ static const char *cmd_rule_perf_time(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_perf_time: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -1874,6 +1936,11 @@ static const char *cmd_conn_read_state_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_conn_read_state_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -1925,6 +1992,11 @@ static const char *cmd_conn_write_state_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_conn_write_state_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -1966,6 +2038,11 @@ static const char *cmd_request_body_inmemory_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_body_inmemory_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -1985,6 +2062,11 @@ static const char *cmd_request_body_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_body_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -2004,6 +2086,11 @@ static const char *cmd_request_body_no_files_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_body_no_files_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -2023,6 +2110,11 @@ static const char *cmd_request_body_json_depth_limit(cmd_parms *cmd, void *_dcfg assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_body_json_depth_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -2061,6 +2153,11 @@ static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_body_access: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) dcfg->reqbody_access = 1; @@ -2088,6 +2185,11 @@ static const char *cmd_request_intercept_on_error(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_intercept_on_error: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) dcfg->reqintercept_oe = 1; @@ -2104,6 +2206,11 @@ static const char *cmd_request_encoding(cmd_parms *cmd, void *_dcfg, const char *p1) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_request_encoding: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; /* ENH Validate encoding */ @@ -2118,6 +2225,11 @@ static const char *cmd_response_body_access(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_response_body_access: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) dcfg->resbody_access = 1; @@ -2135,6 +2247,11 @@ static const char *cmd_response_body_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_response_body_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit; @@ -2158,6 +2275,11 @@ static const char *cmd_response_body_limit_action(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_response_body_limit_action: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (dcfg->is_enabled == MODSEC_DETECTION_ONLY) { @@ -2190,6 +2312,11 @@ static const char *cmd_resquest_body_limit_action(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_resquest_body_limit_action: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (dcfg->is_enabled == MODSEC_DETECTION_ONLY) { @@ -2212,6 +2339,11 @@ static const char *cmd_response_body_mime_type(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(_p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_response_body_mime_type: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; char *p1 = apr_pstrdup(cmd->pool, _p1); @@ -2231,6 +2363,11 @@ static const char *cmd_response_body_mime_types_clear(cmd_parms *cmd, void *_dcfg) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_response_body_mime_types_clear: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->of_mime_types_cleared = 1; @@ -2258,6 +2395,11 @@ static const char *cmd_rule_update_target_by_id(cmd_parms *cmd, void *_dcfg, { assert(cmd != NULL); assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_update_target_by_id: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -2296,6 +2438,11 @@ static const char *cmd_rule_update_target_by_tag(cmd_parms *cmd, void *_dcfg, { assert(cmd != NULL); assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_update_target_by_tag: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -2333,6 +2480,11 @@ static const char *cmd_rule_update_target_by_msg(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_update_target_by_msg: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -2363,6 +2515,11 @@ static const char *cmd_sever_conn_filters_engine(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_sever_conn_filters_engine: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) @@ -2391,6 +2548,11 @@ static const char *cmd_rule_engine(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_engine: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) @@ -2421,6 +2583,11 @@ static const char *cmd_remote_rules_fail(cmd_parms *cmd, void *_dcfg, const char assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_remote_rules_fail: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strncasecmp(p1, "warn", 4) == 0) @@ -2447,6 +2614,11 @@ static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1, assert(_dcfg != NULL); assert(p1 != NULL); char *error_msg = NULL; + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_remote_rules: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; #ifdef WITH_REMOTE_RULES int crypto = 0; @@ -2536,6 +2708,11 @@ static const char *cmd_status_engine(cmd_parms *cmd, void *_dcfg, const char *p1 static const char *cmd_rule_inheritance(cmd_parms *cmd, void *_dcfg, int flag) { assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_inheritance: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->rule_inheritance = flag; return NULL; @@ -2561,6 +2738,11 @@ static const char *cmd_rule_remove_by_id(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_remove_by_id: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception* re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); if (re == NULL) { @@ -2594,6 +2776,11 @@ static const char *cmd_rule_remove_by_tag(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_remove_by_tag: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -2621,6 +2808,11 @@ static const char *cmd_rule_remove_by_msg(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_remove_by_msg: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -2683,6 +2875,11 @@ static const char *cmd_tmp_dir(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_tmp_dir: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "none") == 0) dcfg->tmp_dir = NULL; @@ -2696,6 +2893,11 @@ static const char *cmd_upload_dir(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_upload_dir: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "none") == 0) dcfg->upload_dir = NULL; @@ -2710,6 +2912,11 @@ static const char *cmd_upload_file_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_upload_file_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "default") == 0) { @@ -2728,6 +2935,11 @@ static const char *cmd_upload_filemode(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_upload_filemode: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "default") == 0) { @@ -2751,6 +2963,11 @@ static const char *cmd_upload_keep_files(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_upload_keep_files: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) { @@ -2774,6 +2991,11 @@ static const char *cmd_upload_save_tmp_files(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_upload_save_tmp_files: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) @@ -2798,6 +3020,11 @@ static const char *cmd_web_app_id(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_web_app_id: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; /* ENH enforce format (letters, digits, ., _, -) */ @@ -2811,6 +3038,11 @@ static const char *cmd_sensor_id(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_sensor_id: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; /* ENH enforce format (letters, digits, ., _, -) */ @@ -2834,6 +3066,11 @@ static const char *cmd_xml_external_entity(cmd_parms *cmd, void *_dcfg, const ch assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_xml_external_entity: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) { dcfg->xml_external_entity = 1; @@ -2862,6 +3099,11 @@ static const char *cmd_hash_engine(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_engine: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) { dcfg->hash_is_enabled = HASH_ENABLED; @@ -2890,6 +3132,11 @@ static const char *cmd_hash_param(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_param: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->crypto_param_name = p1; @@ -2913,6 +3160,11 @@ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, co assert(_dcfg != NULL); assert(_p1 != NULL); assert(_p2 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_key: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; char *p1 = NULL; @@ -2954,6 +3206,11 @@ static const char *cmd_hash_method_pm(cmd_parms *cmd, void *_dcfg, assert(_dcfg != NULL); assert(p1 != NULL); assert(p2 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_method_pm: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(hash_method)); const char *_p2 = apr_pstrdup(cmd->pool, p2); @@ -3047,6 +3304,11 @@ static const char *cmd_hash_method_rx(cmd_parms *cmd, void *_dcfg, assert(_dcfg != NULL); assert(p1 != NULL); assert(p2 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_method_rx: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(hash_method)); const char *_p2 = apr_pstrdup(cmd->pool, p2); @@ -3116,6 +3378,11 @@ static const char *cmd_httpBl_key(cmd_parms *cmd, void *_dcfg, const char *p1) assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_httpBl_key: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->httpBlkey = p1; @@ -3285,6 +3552,11 @@ static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_cache_transformations: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; if (strcasecmp(p1, "on") == 0) diff --git a/apache2/re.c b/apache2/re.c index 47a390e0..54d7db8d 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -2385,6 +2385,11 @@ msre_rule *msre_rule_create(msre_ruleset *ruleset, int type, assert(ruleset != NULL); assert(args != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, ruleset->mp, NULL, "msre_rule_create: error_msg is NULL"); + return NULL; + } msre_rule *rule; char *my_error_msg; const char *argsp; diff --git a/apache2/re_operators.c b/apache2/re_operators.c index 73b0d8fb..b2f3cc79 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -106,6 +106,11 @@ static int msre_op_nomatch_execute(modsec_rec *msr, msre_rule *rule, static int msre_op_ipmatch_param_init(msre_rule *rule, char **error_msg) { assert(rule != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_ipmatch_param_init: error_msg is NULL"); + return -1; + } char *param = NULL; int res = 0; @@ -341,6 +346,11 @@ static int msre_op_rsub_param_init(msre_rule *rule, char **error_msg) { assert(rule != NULL); assert(rule->ruleset != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_rsub_param_init: error_msg is NULL"); + return -1; + } #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 0 ap_regex_t *regex; #else @@ -2576,6 +2586,11 @@ static int msre_op_endsWith_execute(modsec_rec *msr, msre_rule *rule, msre_var * assert(rule != NULL); assert(var != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_endsWith_execute: error_msg is NULL"); + return -1; + } const char *match = NULL; const char *target; unsigned int match_length; @@ -3239,6 +3254,10 @@ static int msre_op_verifyCPF_init(msre_rule *rule, char **error_msg) { assert(rule != NULL); assert(rule->ruleset != NULL); assert(error_msg != NULL); + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_verifyCPF_init: error_msg is NULL"); + return -1; + } const char *errptr = NULL; int erroffset; int options = 0; @@ -4122,6 +4141,10 @@ static int msre_op_fuzzy_hash_init(msre_rule *rule, char **error_msg) assert(rule != NULL); assert(rule->ruleset != NULL); assert(error_msg != NULL); + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, ":msre_op_fuzzy_hash_init error_msg is NULL"); + return -1; + } #ifdef WITH_SSDEEP struct fuzzy_hash_param_data *param_data; struct fuzzy_hash_chunk *chunk, *t; @@ -4276,6 +4299,10 @@ static int msre_op_inspectFile_init(msre_rule *rule, char **error_msg) { assert(rule != NULL); assert(rule->ruleset != NULL); assert(error_msg != NULL); + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_inspectFile_init: error_msg is NULL"); + return -1; + } char *filename = (char *)rule->op_param; *error_msg = NULL; From 22a6829690af1ee98814ba5ff64d5b3483ef7ac0 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 14 Aug 2024 18:44:45 +0200 Subject: [PATCH 17/20] added more NULL checks at run-time --- apache2/re_operators.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apache2/re_operators.c b/apache2/re_operators.c index b2f3cc79..6e36300c 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -2659,6 +2659,11 @@ static int msre_op_strmatch_param_init(msre_rule *rule, char **error_msg) { assert(rule != NULL); assert(rule->ruleset != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_strmatch_param_init: error_msg is NULL"); + return -1; + } const apr_strmatch_pattern *compiled_pattern; char *processed = NULL; const char *pattern = rule->op_param; @@ -2691,6 +2696,11 @@ static int msre_op_strmatch_execute(modsec_rec *msr, msre_rule *rule, msre_var * assert(rule != NULL); assert(var != NULL); assert(error_msg != NULL); + // Normally useless code, left to be safe for the moment + if (error_msg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, rule->ruleset->mp, NULL, "msre_op_strmatch_execute: error_msg is NULL"); + return -1; + } apr_strmatch_pattern *compiled_pattern = (apr_strmatch_pattern *)rule->op_param_data; const char *target; unsigned int target_length; From 0066a679114fdc9edc55f52a3d1d5142ccc02739 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 14 Aug 2024 19:00:25 +0200 Subject: [PATCH 18/20] added more NULL checks at run-time --- apache2/apache2_config.c | 36 ++++++++++++++++++++++++++++++++++++ apache2/msc_geo.c | 2 ++ 2 files changed, 38 insertions(+) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 0c79fa3f..6fd5c749 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -2781,6 +2781,10 @@ static const char *cmd_rule_remove_by_tag(cmd_parms *cmd, void *_dcfg, ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_remove_by_tag: _dcfg is NULL"); return NULL; } + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_rule_remove_by_tag: p1 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(rule_exception)); @@ -3165,6 +3169,10 @@ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, co ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_key: _dcfg is NULL"); return NULL; } + if (_p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_key: _p1 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; char *p1 = NULL; @@ -3211,6 +3219,10 @@ static const char *cmd_hash_method_pm(cmd_parms *cmd, void *_dcfg, ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_method_pm: _dcfg is NULL"); return NULL; } + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_method_pm: p1 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; rule_exception *re = apr_pcalloc(cmd->pool, sizeof(hash_method)); const char *_p2 = apr_pstrdup(cmd->pool, p2); @@ -3383,6 +3395,10 @@ static const char *cmd_httpBl_key(cmd_parms *cmd, void *_dcfg, const char *p1) ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_httpBl_key: _dcfg is NULL"); return NULL; } + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_httpBl_key: p1 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->httpBlkey = p1; @@ -3397,6 +3413,11 @@ static const char *cmd_pcre_match_limit(cmd_parms *cmd, { assert(cmd != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_pcre_match_limit: p1 is NULL"); + return NULL; + } long val; if (cmd->server->is_virtual) { @@ -3418,6 +3439,11 @@ static const char *cmd_pcre_match_limit_recursion(cmd_parms *cmd, { assert(cmd != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_pcre_match_limit_recursion: p1 is NULL"); + return NULL; + } long val; if (cmd->server->is_virtual) { @@ -3442,6 +3468,16 @@ static const char *cmd_geo_lookup_db(cmd_parms *cmd, void *_dcfg, { assert(cmd != NULL); assert(p1 != NULL); + assert(_dcfg != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_geo_lookup_db: _dcfg is NULL"); + return NULL; + } + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_geo_lookup_db: p1 is NULL"); + return NULL; + } const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); char *error_msg; directory_config *dcfg = (directory_config *)_dcfg; diff --git a/apache2/msc_geo.c b/apache2/msc_geo.c index e77e4f50..6f60b003 100644 --- a/apache2/msc_geo.c +++ b/apache2/msc_geo.c @@ -12,6 +12,7 @@ * directly using the email address security@modsecurity.org. */ +#include #include "msc_geo.h" @@ -244,6 +245,7 @@ static int field_length(const char *field, int maxlen) */ int geo_init(directory_config *dcfg, const char *dbfn, char **error_msg) { + assert(dcfg != NULL); *error_msg = NULL; if ((dcfg->geo == NULL) || (dcfg->geo == NOT_SET_P)) { From 4b391834ecc91dcb0ea93448edf1ad2508bb2b6e Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 14 Aug 2024 19:09:15 +0200 Subject: [PATCH 19/20] added more NULL checks at run-time --- apache2/apache2_config.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 6fd5c749..54540bd9 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -3141,6 +3141,10 @@ static const char *cmd_hash_param(cmd_parms *cmd, void *_dcfg, const char *p1) ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_param: _dcfg is NULL"); return NULL; } + if (p1 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_param: p1 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; dcfg->crypto_param_name = p1; @@ -3173,6 +3177,10 @@ static const char *cmd_hash_key(cmd_parms *cmd, void *_dcfg, const char *_p1, co ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_key: _p1 is NULL"); return NULL; } + if (_p2 == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_hash_key: _p2 is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; char *p1 = NULL; @@ -3533,6 +3541,11 @@ static const char *cmd_unicode_map(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(p1 != NULL); assert(p2 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_unicode_map: _dcfg is NULL"); + return NULL; + } const char *filename = resolve_relative_path(cmd->pool, cmd->directive->filename, p1); char *error_msg; long val = 0; From 60d07a5547ff18065f127699caac4010ac63c884 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Fri, 16 Aug 2024 09:23:11 +0200 Subject: [PATCH 20/20] added one more NULL check at run-time --- apache2/apache2_config.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 54540bd9..76427653 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -2134,6 +2134,11 @@ static const char *cmd_arguments_limit(cmd_parms *cmd, void *_dcfg, assert(cmd != NULL); assert(_dcfg != NULL); assert(p1 != NULL); + // Normally useless code, left to be safe for the moment + if (_dcfg == NULL) { + ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, cmd->pool, NULL, "cmd_arguments_limit: _dcfg is NULL"); + return NULL; + } directory_config *dcfg = (directory_config *)_dcfg; long int limit;