Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

double free in ModSecurity3 'reading_logs_via_rule_message' example and OWASP CRS #1573

Closed
majordaw opened this issue Sep 26, 2017 · 1 comment
Assignees

Comments

@majordaw
Copy link
Contributor

majordaw commented Sep 26, 2017

Hi,
when I would like to use the OWASP CRS in a multi-threaded environment (e.g: in the reading_logs_via_rule_message example) I get segfaults in the acmp_build_binary_tree(ACMP *parser, acmp_node_t *node) function.

Here is how to reproduce on Ubuntu 16.04/gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4):
git clone https://github.com/SpiderLabs/ModSecurity.git
cd ModSecurity
git checkout origin/v3/master
git submodule init
git submodule update
./build.sh
./configure
MAKEFLAGS=-j4 make
cp modsecurity.conf-recommended examples/reading_logs_via_rule_message/modsec.conf
cd examples/reading_logs_via_rule_message/

Comment out in modsec.conf: SecAuditEngine RelevantOnly

git clone https://github.com/SpiderLabs/owasp-modsecurity-crs.git
cat owasp-modsecurity-crs/crs-setup.conf.example >> modsec.conf
echo 'Include "owasp-modsecurity-crs/rules/*.conf"' >> modsec.conf
./simple_request modsec.conf

There is the gdb stack back-trace:
Thread 3 "simple_request" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffee8fd700 (LWP 31472)]
0x00007ffff60a6428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
54 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007ffff60a6428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007ffff60a802a in __GI_abort () at abort.c:89
#2 0x00007ffff60e87ea in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7ffff6201e98 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#3 0x00007ffff60f137a in malloc_printerr (ar_ptr=, ptr=, str=0x7ffff6201f60 "double free or corruption (fasttop)", action=3) at malloc.c:5006
#4 _int_free (av=, p=, have_lock=0) at malloc.c:3867
#5 0x00007ffff60f553c in __GI___libc_free (mem=) at malloc.c:2968
#6 0x000000000046e296 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:358
#7 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#8 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#9 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#10 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#11 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#12 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#13 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#14 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#15 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#16 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#17 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#18 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#19 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#20 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#21 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#22 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#23 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#24 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#25 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#26 0x000000000046e2f9 in acmp_build_binary_tree (parser=parser@entry=0x9027b0, node=, node=) at utils/acmp.cc:366
#27 0x000000000046ecc1 in acmp_connect_fail_branches (parser=0x9027b0) at utils/acmp.cc:420
#28 acmp_prepare (parser=parser@entry=0x9027b0) at utils/acmp.cc:459
#29 0x000000000046efd1 in acmp_process_quick (acmpt=acmpt@entry=0x7fffee8fabf0, match=match@entry=0x7fffee8fabe8, data=0x7fffee8fb350 "/test.pl", len=8) at utils/acmp.cc:536
#30 0x000000000045c0f1 in modsecurity::operators::Pm::evaluate (this=, transaction=0x7fffe00008c0, rule=0x90c980, input=..., ruleMessage=std::shared_ptr (count 4, weak 0) 0x7fffe0011f70) at operators/pm.cc:90
#31 0x00000000004589f6 in modsecurity::operators::Operator::evaluateInternal (this=0x9020a0, transaction=transaction@entry=0x7fffe00008c0, rule=rule@entry=0x90c980, a="/test.pl", rm=std::shared_ptr (count 4, weak 0) 0x7fffe0011f70) at operators/operator.cc:80
#32 0x000000000042b754 in modsecurity::Rule::executeOperatorAt (this=this@entry=0x90c980, trans=trans@entry=0x7fffe00008c0, key="REQUEST_FILENAME", value="/test.pl", ruleMessage=std::shared_ptr (count 4, weak 0) 0x7fffe0011f70) at rule.cc:294
#33 0x0000000000431dcb in modsecurity::Rule::evaluate (this=this@entry=0x90c980, trans=trans@entry=0x7fffe00008c0, ruleMessage=std::shared_ptr (count 4, weak 0) 0x7fffe0011f70) at rule.cc:772
#34 0x0000000000427637 in modsecurity::Rules::evaluate (this=0x7bc7a0, phase=phase@entry=3, transaction=transaction@entry=0x7fffe00008c0) at rules.cc:219
#35 0x0000000000411091 in modsecurity::Transaction::processRequestBody (this=this@entry=0x7fffe00008c0) at transaction.cc:812
#36 0x000000000040b180 in process_request (data=) at ../../examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h:91
#37 0x00007ffff7bc16ba in start_thread (arg=0x7fffee8fd700) at pthread_create.c:333
#38 0x00007ffff61783dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb)

The corruption seems to occur when more threads (Transactions) want to evaluate the same Rule in the Rules object at the same time. It seems to me that some rule or operator build a bin-tree at their first evaluation which can lead to a race condition.

When I do only one Transaction at one time before the start of the other threads the problem disappear.

    +process_request((void *)&dms);
    +
     for (i = 0; i < NUM_THREADS; i++) {
         pthread_create(&threads[i], NULL, process_request, (void *)&dms);
         //process_request((void *)&dms);
@zimmerle zimmerle self-assigned this Sep 26, 2017
@zimmerle
Copy link
Contributor

Hi @majordaw,

Thanks for the report. Recently we have added this flag called: --enable-mutex-on-pm. You can use this flag in the ./configure script to make acmp threes thread safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants