Checks HTTP code after performing a resource download

As reported by Walter Hop on our dev- mailing list, remote resource download
was not validating the HTTP code, parsing errors pages as resources. This
commit fix  this issue, from now one HTTP error codes will be verified and
treated as errors. Operators are now dealing well with empty values that may
be produced in consequence of a download error.

Author: Felipe Zimmerle

apache2/msc_remote_rules.c

@@ -247,6 +247,7 @@ int msc_remote_download_content(apr_pool_t *mp, const char *uri, const char *key
     char *beacon_str = NULL;
     char *beacon_apr = NULL;
     int beacon_str_len = 0;
+    int ret = 0;
 
     chunk->size = 0;
 
@@ -315,11 +316,12 @@ int msc_remote_download_content(apr_pool_t *mp, const char *uri, const char *key
         /* we pass our 'chunk' struct to the callback function */
         curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)chunk);
 
-        /* some servers don't like requests that are made without a user-agent
-           field, so we provide one */
         curl_easy_setopt(curl, CURLOPT_USERAGENT, "modesecurity");
         curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers_chunk);
 
+        /* We want Curl to return error in case there is an HTTP error code */
+        curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
+
         res = curl_easy_perform(curl);
 
         if (res != CURLE_OK)
@@ -330,24 +332,27 @@ int msc_remote_download_content(apr_pool_t *mp, const char *uri, const char *key
                          "Failed to download \"%s\" error: %s ",
                          uri, curl_easy_strerror(res));
 
-                 return -2;
+                 ret = -2;
+                 goto failed;
             }
             else
             {
                 *error_msg = apr_psprintf(mp, "Failed to download \"%s\" " \
                     "error: %s ",
                     uri, curl_easy_strerror(res));
 
-                return -1;
+                ret = -1;
+                goto failed;
             }
         }
 
         curl_slist_free_all(headers_chunk);
     }
 
+failed:
     curl_easy_cleanup(curl);
 
-    return 0;
+    return ret;
 #else
     return -3;
 #endif

apache2/re_operators.c

@@ -266,8 +266,12 @@ static int msre_op_ipmatchFromFile_execute(modsec_rec *msr, msre_rule *rule,
     else
         *error_msg = NULL;
 
-    if(rtree == NULL) {
-        msr_log(msr, 1, "ipMatchFromFile Internal Error: tree value is null.");
+    if (rtree == NULL)
+    {
+        if (msr->txcfg->debuglog_level >= 4)
+        {
+            msr_log(msr, 1, "ipMatchFromFile: tree value is null.");
+        }
         return 0;
     }
 
@@ -1388,6 +1392,16 @@ static int msre_op_pm_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c
     /* Are we supposed to capture subexpressions? */
     capture = apr_table_get(rule->actionset->actions, "capture") ? 1 : 0;
 
+    if (rule->op_param_data == NULL)
+    {
+        if (msr->txcfg->debuglog_level >= 4)
+        {
+            msr_log(msr, 1, "ACMPTree is null.");
+        }
+
+        return 0;
+    }
+
     pt.parser = (ACMP *)rule->op_param_data;
     pt.ptr = NULL;