Closed
Description
Hi,
I probably found a bug with the order of files loaded when the files are described with a wildcard.
I fixed this. The commit description contains more information.
UnfortunateIy, I do not understand anything with the pull request system, so I put the patche on the git format on my web site here:
http://www.arpalert.org/0001-Fix-bug-when-load-files.patch
And a copy/paste here (I have some doubts about this method)
From 38d327581e2b7dd75809e19855dc5c110ad0a21f Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <[email protected]>
Date: Sat, 10 Dec 2016 18:40:01 +0100
Subject: [PATCH] Fix bug when load files
The order of files loaded is very immortant. Typically the "SecRuleRemoveById"
rules must be loaded after the concerned rules. Today, when I try to load
AWASP rules with a wildcard directive, an strace shows the following
order (I remove useles syscall and I short the path):
strace -e open ....
[...]
open("./modsecurity.conf", O_RDONLY|O_CLOEXEC) = 4
[...]
open("./rules/modsecurity_crs_90_exceptions.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_47_skip_outbound_checks.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_46_slr_et_wordpress_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
[...]
open("./rules/modsecurity_crs_46_slr_et_phpbb_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
[...]
open("./rules/modsecurity_crs_46_slr_et_joomla_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
[...]
open("./rules/modsecurity_crs_41_xss_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_41_sql_injection_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_40_generic_attacks.conf", O_RDONLY|O_CLOEXEC) = 5
[...]
open("./rules/modsecurity_crs_30_http_policy.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_23_request_limits.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_21_protocol_anomalies.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_20_protocol_violations.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_13_xml_enabler_extended.conf", O_RDONLY|O_CLOEXEC) = 5
open("./rules/modsecurity_crs_10_ignore_static_extended.conf", O_RDONLY|O_CLOEXEC) = 5
[...]
"modsecurity.conf" contains:
Include ./rules/*.conf
This trace show a reverse alphabetical order. In this example, my exceptions
file (modsecurity_crs_90_exceptions.conf) is loaded first, and the directives
"SecRuleRemoveById" are ignored.
This behaviour is explained by the loader algorithm:
1 -> push first config file in the "ari" pool
2 -> while the "ari" pool contains entries:
a -> pop a config file and process it
b -> if the file contains an include with wilcard
A -> load all the corresponding files in an array
B -> alphabetically sort the array anf push each entries in the "ari"
The included file are alphabetically sorted and pushed in the array, so
the algorithm "pop" the file from the array, so the "pop" take the last
entry and not the first.
The APR library doesn't provides a method for fetching the first entry of
an array, so in order to use the "pop" method, this patch just reverse the
alphabetical sort. The files are now push in the reverse order, and the "pop"
system works fine.
This patch must be backported on the stable version.
---
standalone/config.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/standalone/config.c b/standalone/config.c
index 800d5b4..978499e 100644
--- a/standalone/config.c
+++ b/standalone/config.c
@@ -742,12 +742,19 @@ AP_DECLARE(char *) ap_make_full_path(apr_pool_t *a, const char *src1,
return path;
}
-static int fname_alphasort(const void *fn1, const void *fn2)
+/* Note that the file are pushed in the "ari" lifo in alphabetically
+ * order. There are read using a "pop". The "pop" takes the last file.
+ * We must follow the alphabetical order for loading file, so in
+ * order to use the "pop" function (APR seems to not provide a fetch
+ * method for the first entry), this function sort in reverse alphabetic
+ * order.
+ */
+static int fname_reversealphasort(const void *fn1, const void *fn2)
{
const fnames *f1 = fn1;
const fnames *f2 = fn2;
- return strcmp(f1->fname,f2->fname);
+ return strcmp(f2->fname,f1->fname);
}
int fnmatch_test(const char *pattern)
@@ -840,7 +847,7 @@ static const char *process_resource_config_nofnmatch(const char *fname,
apr_dir_close(dirp);
if (candidates->nelts != 0) {
qsort((void *) candidates->elts, candidates->nelts,
- sizeof(fnames), fname_alphasort);
+ sizeof(fnames), fname_reversealphasort);
/*
* Now recurse these... we handle errors and subdirectories
@@ -941,7 +948,7 @@ static const char *process_resource_config_fnmatch(const char *path,
const char *error;
qsort((void *) candidates->elts, candidates->nelts,
- sizeof(fnames), fname_alphasort);
+ sizeof(fnames), fname_reversealphasort);
/*
* Now recurse these... we handle errors and subdirectories
--
1.7.10.4