Project

General

Profile

Actions
Copy link

Bug #970

closed

AC memory read error

Added by Victor Julien about 11 years ago. Updated about 11 years ago.

Status:
Closed
Priority:
High
Assignee:
Anoop Saldanha
Target version:
2.0beta2
Affected Versions:
Effort:
Difficulty:
Label:

Description

Using clang 3.2's -fsanitize=address option results in an error in AC.

CC=clang CFLAGS="-Werror -O0 -ggdb -fsanitize=address" ./configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var

The error:

=================================================================
==6918== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fdc27670d3d at pc 0x2299721 bp 0x7fdbedbeba50 sp 0x7fdbedbeba10
READ of size 1 at 0x7fdc27670d3d thread T3
    #0 0x2299720 in __interceptor_memcmp ??:?
    #1 0x1f2e7d5 in SCACSearch /home/victor/dev/oisf/src/util-mpm-ac.c:1271
    #2 0x11617db in HttpHeaderPatternSearch /home/victor/dev/oisf/src/detect-engine-mpm.c:360
    #3 0xee2798 in DetectEngineRunHttpHeaderMpm /home/victor/dev/oisf/src/detect-engine-hhd.c:203
    #4 0xb196f9 in DetectMpmPrefilter /home/victor/dev/oisf/src/detect.c:862
    #5 0xb0c1a0 in SigMatchSignatures /home/victor/dev/oisf/src/detect.c:1295
    #6 0xaefeb0 in Detect /home/victor/dev/oisf/src/detect.c:1696
    #7 0x1d75409 in TmThreadsSlotVarRun /home/victor/dev/oisf/src/tm-threads.c:542
    #8 0x1d7a4c3 in TmThreadsSlotVar /home/victor/dev/oisf/src/tm-threads.c:789
    #9 0x229ff9a in _ZN6__asan10AsanThread11ThreadStartEv ??:?
0x7fdc27670d3d is located 3 bytes to the left of 92-byte region [0x7fdc27670d40,0x7fdc27670d9c)
allocated by thread T3 here:
    #0 0x229d1a5 in realloc ??:?
    #1 0xee4fa9 in DetectEngineHHDGetBufferForTX /home/victor/dev/oisf/src/detect-engine-hhd.c:160
    #2 0xee2589 in DetectEngineRunHttpHeaderMpm /home/victor/dev/oisf/src/detect-engine-hhd.c:195
    #3 0xb196f9 in DetectMpmPrefilter /home/victor/dev/oisf/src/detect.c:862
    #4 0xb0c1a0 in SigMatchSignatures /home/victor/dev/oisf/src/detect.c:1295
    #5 0xaefeb0 in Detect /home/victor/dev/oisf/src/detect.c:1696
    #6 0x1d75409 in TmThreadsSlotVarRun /home/victor/dev/oisf/src/tm-threads.c:542
    #7 0x1d7a4c3 in TmThreadsSlotVar /home/victor/dev/oisf/src/tm-threads.c:789
    #8 0x229ff9a in _ZN6__asan10AsanThread11ThreadStartEv ??:?
Thread T3 created by T0 here:
    #0 0x2299284 in __interceptor_pthread_create ??:?
    #1 0x1d8f49d in TmThreadSpawn /home/victor/dev/oisf/src/tm-threads.c:1882
    #2 0x1abee62 in RunModeFilePcapAutoFp /home/victor/dev/oisf/src/runmode-pcap-file.c:427
    #3 0x1aca638 in RunModeDispatch /home/victor/dev/oisf/src/runmodes.c:336
    #4 0x1d28e82 in main /home/victor/dev/oisf/src/suricata.c:2011
    #5 0x7fdc3ca1fea4 in ?? ??:0
Shadow byte and word:
  0x1ffb84ece1a7: fa
  0x1ffb84ece1a0: fa fa fa fa fa fa fa fa
More shadow bytes:
  0x1ffb84ece180: fa fa fa fa fa fa fa fa
  0x1ffb84ece188: 00 00 00 00 00 00 00 00
  0x1ffb84ece190: 00 00 00 fb fb fb fb fb
  0x1ffb84ece198: fa fa fa fa fa fa fa fa
=>0x1ffb84ece1a0: fa fa fa fa fa fa fa fa
  0x1ffb84ece1a8: 00 00 00 00 00 00 00 00
  0x1ffb84ece1b0: 00 00 00 04 fb fb fb fb
  0x1ffb84ece1b8: fa fa fa fa fa fa fa fa
  0x1ffb84ece1c0: fa fa fa fa fa fa fa fa
Stats: 1234381M malloced (597128M for red zones) by 3223190 calls
Stats: 1233843M realloced by 333615 calls
Stats: 1234167M freed by 2601897 calls
Stats: 1234134M really freed by 2381312 calls
Stats: 1275M (326525 full pages) mmaped in 1299 calls
  mmaps   by size class: 7:708435; 8:298862; 9:71610; 10:49567; 11:44880; 12:1152; 13:1984; 14:352; 15:1248; 16:152; 17:336; 18:214; 19:131; 20:87; 21:36; 22:21; 23:12; 24:5; 25:3; 26:3;
  mallocs by size class: 7:1863535; 8:664223; 9:291168; 10:62376; 11:132304; 12:2638; 13:4771; 14:5318; 15:9709; 16:15171; 17:18996; 18:25956; 19:20069; 20:14568; 21:8211; 22:14629; 23:17889; 24:24448; 25:17341; 26:9870;
  frees   by size class: 7:1532734; 8:439101; 9:277373; 10:15771; 11:130976; 12:2234; 13:3434; 14:5109; 15:8467; 16:15085; 17:18662; 18:25949; 19:20066; 20:14560; 21:8208; 22:14625; 23:17886; 24:24447; 25:17341; 26:9869;
  rfrees  by size class: 7:1439089; 8:367509; 9:226023; 10:13234; 11:130659; 12:1730; 13:2951; 14:4993; 15:8467; 16:15045; 17:18661; 18:25949; 19:20066; 20:14560; 21:8208; 22:14625; 23:17886; 24:24447; 25:17341; 26:9869;
Stats: malloc large: 196857 small slow: 31443
==6918== ABORTING

What appears to be happening is that there is an int-underflow in AC. Adding this check:

--- a/src/util-mpm-ac.c
+++ b/src/util-mpm-ac.c
@@ -1268,6 +1268,9 @@ uint32_t SCACSearch(MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx,
                 uint32_t k;
                 for (k = 0; k < no_of_entries; k++) {
                     if (pids[k] & 0xFFFF0000) {
+                        uint32_t offset = i - pid_pat_list[pids[k] & 0x0000FFFF].patlen + 1;
+                        BUG_ON(offset > buflen);
+
                         if (SCMemcmp(pid_pat_list[pids[k] & 0x0000FFFF].cs,
                                      buf + i - pid_pat_list[pids[k] & 0x0000FFFF].patlen + 1,
                                      pid_pat_list[pids[k] & 0x0000FFFF].patlen) != 0) {

Results in:

Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `./src/suricata -l tmp/ -c suricata.yaml -r /home/victor/sync/pcap/sandnet.pcap'.
Program terminated with signal 6, Aborted.
#0  0x00007f82a39a1425 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007f82a39a1425 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007f82a39a4b8b in __GI_abort () at abort.c:91
#2  0x00007f82a399a0ee in __assert_fail_base (fmt=<optimized out>, assertion=0xadb62e "!(offset > buflen)", file=0xadb3b0 "util-mpm-ac.c", line=<optimized out>, function=<optimized out>) at assert.c:94
#3  0x00007f82a399a192 in __GI___assert_fail (assertion=0xadb62e "!(offset > buflen)", file=0xadb3b0 "util-mpm-ac.c", line=1272, function=0xadbb67 <__PRETTY_FUNCTION__.14927> "SCACSearch") at assert.c:103
#4  0x0000000000972ebd in SCACSearch (mpm_ctx=0x36e9d80, mpm_thread_ctx=0x7f8290002220, pmq=0x7f8290002240, 
    buf=0x7f829003cfb0 "User-Agent: Microsoft Internet Explorer\r\nHost: orav4abustorabe.com\r\nConnection: Keep-Alive\r\n\202\177", buflen=92) at util-mpm-ac.c:1272
#5  0x0000000000692f0f in HttpHeaderPatternSearch (det_ctx=0x7f8290002170, headers=0x7f829003cfb0 "User-Agent: Microsoft Internet Explorer\r\nHost: orav4abustorabe.com\r\nConnection: Keep-Alive\r\n\202\177", headers_len=92, 
    flags=6 '\006') at detect-engine-mpm.c:360
#6  0x0000000000621b38 in DetectEngineRunHttpHeaderMpm (det_ctx=0x7f8290002170, f=0x34d5640, htp_state=0x7f8290036410, flags=6 '\006', tx=0x7f8290048bc0, idx=0) at detect-engine-hhd.c:203
#7  0x0000000000563fb3 in DetectMpmPrefilter (de_ctx=0x35f7f50, det_ctx=0x7f8290002170, smsg=0x36473d0, p=0x2fefe00, flags=6 '\006', alproto=1, alstate=0x7f8290036410, sms_runflags=0x7f829bffd396 "\001'") at detect.c:862
#8  0x00000000005679f4 in SigMatchSignatures (th_v=0x35fc450, de_ctx=0x35f7f50, det_ctx=0x7f8290002170, p=0x2fefe00) at detect.c:1295
#9  0x000000000056a3b2 in Detect (tv=0x35fc450, p=0x2fefe00, data=0x7f8290002170, pq=0x35fc6c0, postpq=0x0) at detect.c:1696
#10 0x00000000009123d2 in TmThreadsSlotVarRun (tv=0x35fc450, p=0x2fefe00, slot=0x35fc540) at tm-threads.c:542
#11 0x0000000000913703 in TmThreadsSlotVar (td=0x35fc450) at tm-threads.c:789
#12 0x00007f82a46d4e9a in start_thread (arg=0x7f829bfff700) at pthread_create.c:308
#13 0x00007f82a3a5eccd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#14 0x0000000000000000 in ?? ()
(gdb) f 4
#4  0x0000000000972ebd in SCACSearch (mpm_ctx=0x36e9d80, mpm_thread_ctx=0x7f8290002220, pmq=0x7f8290002240, 
    buf=0x7f829003cfb0 "User-Agent: Microsoft Internet Explorer\r\nHost: orav4abustorabe.com\r\nConnection: Keep-Alive\r\n\202\177", buflen=92) at util-mpm-ac.c:1272
1272                            BUG_ON(offset > buflen);
(gdb) print offset
$1 = 4294967293

The value of offset here is initialized by "i - pid_pat_list[pids[k] & 0x0000FFFF].patlen + 1". I suspect that the result of this is a signed int (i is signed), and thus it is -3. So AC will look 3 bytes before the "buf" pointer. This nicely matches clang's error "0x7fdc27670d3d is located 3 bytes to the left of 92-byte region [0x7fdc27670d40,0x7fdc27670d9c)"

With my patch a gcc compiled binary will abort, so the issue can be reproduced w/o clang.

Will send pcap and rules to reproduce through email.

Actions
Copy link
 #1

Updated by Victor Julien about 11 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100
  • Private changed from Yes to No

Addressed by:

commit d2ea799d38ab37fb143c030fd14ee571d335f4e8
Author: Anoop Saldanha <anoopsaldanha@gmail.com>
Date:   Mon Sep 23 15:23:12 2013 +0530

    fix for bug #970.

    Content strings that are a duplicate of a pattern from another sig, but
    have a fast_pattern chop being applied, would end up being assigned the
    same pattern id as the duplicate string.  But the string supplied to the
    mpm would be the chopped string, which might result in the state_table
    output_state content entry being over-riden by the the fuller string at
    the final state of the smaller content length, because of which during a
    match we might end up inspecting the search buffer against the fuller
    content pattern, instead of the chopped pattern, which would end up being
    an inspection beyond the buffer bounds.

commit da75db93302eee74288f9d532c167d7964051c4a
Author: Anoop Saldanha <anoopsaldanha@gmail.com>
Date:   Mon Sep 23 19:54:24 2013 +0530

    Unittest to display bug #970.

Thanks Anoop.

Actions
Copy link

Also available in: Atom PDF