Merge pull request #3193 from marcstern/v2/pr/useless

Removed useless code
owasp-modsecurity · Aug 14, 2024 · 277e7e2 · 277e7e2
2 parents 8cfb911 + 692710c
commit 277e7e2
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 105 deletions.
diff --git 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
@@ -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
@@ -461,37 +461,30 @@ 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);
 
         msr->stream_input_data = (char *)stream_input_body;
     }
 
     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
@@ -2473,28 +2473,16 @@ int msc_headers_to_buffer(const apr_array_header_t *arr, char *buffer,
 
 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
@@ -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
@@ -1251,27 +1251,26 @@ 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;
         char *p1 = NULL, *p2 = NULL;
         char *savedptr = NULL;
 
         p1 = apr_strtok(value,";",&savedptr);
-
         p2 = apr_strtok(NULL,";",&savedptr);
 
         if (msr->txcfg->debuglog_level >= 4) {
@@ -1282,24 +1281,23 @@ 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;
         char *p1 = NULL, *p2 = NULL;
         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
@@ -617,43 +617,31 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
     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';
 
         var->value_len = size;
         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;
 #ifdef MSC_LARGE_STREAM_INPUT
         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,16 +1667,14 @@ 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)
         return -1;
 
     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
@@ -2454,7 +2454,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;