diff --git a/CHANGES b/CHANGES index 80d8520c47..9e7acc36d2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ DD mmm YYYY - 2.9.x (to be released) ------------------- + * Many NULL pointers checks + [PR #3120 @marcstern] * Enhance logging [Issue #3107 - @marcstern] * Fix possible segfault in collection_unpack diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index 5d2ef85bd9..405b649ae4 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 3287eeff2e..4f73d15a7a 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 ba8bdfd416..c01a37ed8a 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); @@ -479,16 +479,15 @@ apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buf return -1; } - memset(msr->stream_input_data, 0, msr->stream_input_length+1); - - if(first_pkt) { + 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) { + if (data) { free(data); data = NULL; } diff --git a/apache2/msc_util.c b/apache2/msc_util.c index fd318a087a..7a3105a100 100644 --- a/apache2/msc_util.c +++ b/apache2/msc_util.c @@ -2480,8 +2480,6 @@ int read_line(char *buf, int len, FILE *fp) return -1; } - memset(buf, '\0', len*sizeof(char)); - if (fgets(buf, len, fp) == NULL) { *buf = '\0'; diff --git a/apache2/re.c b/apache2/re.c index 8e69f5bafa..4687a17dbb 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 36f898dd23..c9b5364f34 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -1764,7 +1764,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 178c7b7bb0..3997213321 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -619,7 +619,6 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, if(msr->stream_output_data != NULL && output_body == 1) { - memset(msr->stream_output_data, 0x0, msr->stream_output_length); free(msr->stream_output_data); msr->stream_output_data = NULL; msr->stream_output_length = 0; @@ -631,7 +630,6 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, } msr->stream_output_length = size; - memset(msr->stream_output_data, 0x0, size+1); msr->of_stream_changed = 1; @@ -643,7 +641,6 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, } if(msr->stream_input_data != NULL && input_body == 1) { - memset(msr->stream_input_data, 0x0, msr->stream_input_length); free(msr->stream_input_data); msr->stream_input_data = NULL; msr->stream_input_length = 0; @@ -1584,10 +1581,8 @@ static const char *gsb_replace_tpath(apr_pool_t *pool, const char *domain, int l url = apr_palloc(pool, len + 1); data = apr_palloc(pool, len + 1); - memset(data, 0, len+1); - memset(url, 0, len+1); - memcpy(url, domain, len); + url[len] = 0; while(( pos = strstr(url , "/./" )) != NULL) { match = 1; @@ -1690,8 +1685,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) @@ -1699,7 +1692,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 5aa7589a2b..dee23af24b 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;