Fix invalid read in OS_Regex

[JuantAldea]

Related issue
#3813

Description

There's an invalid read in OS_Regex that can be triggered by trying to match pattern that ends with an escaped character, with a string that matches the preceding pattern except the escaped character, e.g, pattern ab\s with string abc

# ossec-regex ab\\s
==3986== Memcheck, a memory error detector
==3986== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3986== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==3986== Command: ./ossec-regex ab\\s
==3986== 
ab 
+OSRegex_Execute: ab 
+OS_Regex       : ab 
abc
==3986== Invalid read of size 1
==3986==    at 0x1191FE: _OS_Regex (os_regex_execute.c:358)
==3986==    by 0x118B71: OSRegex_Execute_ex (os_regex_execute.c:148)
==3986==    by 0x117FD0: OSRegex_Execute (os_regex_execute.c:30)
==3986==    by 0x10BB00: main (ossec-regex.c:69)
==3986==  Address 0x510e185 is 0 bytes after a block of size 5 alloc'd
==3986==    at 0x483874F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==3986==    by 0x4FAAFE9: strdup (strdup.c:42)
==3986==    by 0x116D0E: OSRegex_Compile (os_regex_compile.c:235)
==3986==    by 0x10BA23: main (ossec-regex.c:53)
==3986== 
==3986== Invalid read of size 1
==3986==    at 0x1191FE: _OS_Regex (os_regex_execute.c:358)
==3986==    by 0x118B71: OSRegex_Execute_ex (os_regex_execute.c:148)
==3986==    by 0x117FD0: OSRegex_Execute (os_regex_execute.c:30)
==3986==    by 0x116480: OS_Regex (os_regex.c:30)
==3986==    by 0x10BB99: main (ossec-regex.c:76)
==3986==  Address 0x510f225 is 0 bytes after a block of size 5 alloc'd
==3986==    at 0x483874F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==3986==    by 0x4FAAFE9: strdup (strdup.c:42)
==3986==    by 0x116D0E: OSRegex_Compile (os_regex_compile.c:235)
==3986==    by 0x116466: OS_Regex (os_regex.c:29)
==3986==    by 0x10BB99: main (ossec-regex.c:76

# ossec-regex ab\\s
ab 
+OSRegex_Execute: ab 
+OS_Regex       : ab 
abc
=================================================================
==10048==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000095 at pc 0x564060912f41 bp 0x7ffc57405700 sp 0x7ffc574056f0
READ of size 1 at 0x602000000095 thread T0
    #0 0x564060912f40 in _OS_Regex os_regex/os_regex_execute.c:358
    #1 0x564060911ed0 in OSRegex_Execute_ex os_regex/os_regex_execute.c:148
    #2 0x56406090ffb6 in OSRegex_Execute os_regex/os_regex_execute.c:30
    #3 0x564060903bc0 in main util/ossec-regex.c:69
    #4 0x7f6cb69dab6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
    #5 0x564060903879 in _start (/home/juen/wazuh-vagrant/wazuh/src/ossec-regex+0x3879)

0x602000000095 is located 0 bytes to the right of 5-byte region [0x602000000090,0x602000000095)
allocated by thread T0 here:
    #0 0x7f6cb72c216d in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x9516d)
    #1 0x56406090ecf4 in OSRegex_Compile os_regex/os_regex_compile.c:235
    #2 0x564060903ae3 in main util/ossec-regex.c:53
    #3 0x7f6cb69dab6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

SUMMARY: AddressSanitizer: heap-buffer-overflow os_regex/os_regex_execute.c:358 in _OS_Regex
Shadow bytes around the buggy address:
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff8000: fa fa 05 fa fa fa fd fa fa fa 00 00 fa fa 00 fa
=>0x0c047fff8010: fa fa[05]fa fa fa 00 fa fa fa 05 fa fa fa fd fa
  0x0c047fff8020: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 05 fa
  0x0c047fff8030: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd
  0x0c047fff8040: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fff8050: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8060: fa fa fd fa fa fa 04 fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==10048==ABORTING

The invalid read is located at

else if ((*(pt + 3) == '\0') && (_regex_matched == 1) && (r_code)) {

The end of string (\0) of the patterns is located at pt+2 but the code tries to find it at pt+3 resulting in an invalid read.

Fix & rationale

A fix is to change the affected line to

else if ((*(pt + 2) == '\0' || *(pt + 3) == '\0')  && (_regex_matched == 1) && (r_code)) {

This fix

• makes no difference in cases where the end of the string is located at pt+3, as pt+2 would be different than the end of the string, otherwise pt+2 would be the end of the string , and
• fixes those where the end of the string is located at pt+2, as the short-circuit evaluation will prevent *(pt+3) from being evaluated.

Moreover, accesing to pt + 1 cannot provoke an invalid read, see:

if (*pt == BACKSLASH) {
    if (Regex((uchar) * (pt + 1), (uchar)*st)) {
        [...]
    } else if ((*(pt + 3) == '\0') && (_regex_matched == 1) && (r_code)) { <- Invalid read
        [...]
    }
    [...]
}

*(pt + 0) is known to be a BACKSLASH, and *(pt + 1) must be a character, otherwise the regex would have not compiled.

Q.E.D

Tests

LD_LIBRARY_PATH=$LD_LIBRARY_PATH:.. valgrind ./ossec-regex ab\\s
==8197== Memcheck, a memory error detector
==8197== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==8197== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==8197== Command: ./ossec-regex ab\\s
==8197== 
ab
ab 
+OSRegex_Execute: ab 
+OS_Regex       : ab 
abc 
abc
^C==8197== 
==8197== Process terminating with default action of signal 2 (SIGINT)
==8197==    at 0x501AF81: read (read.c:26)
==8197==    by 0x4F9DE4F: _IO_file_underflow@@GLIBC_2.2.5 (fileops.c:524)
==8197==    by 0x4F9F181: _IO_default_uflow (genops.c:362)
==8197==    by 0x4F911F9: _IO_getline_info (iogetline.c:60)
==8197==    by 0x4F901AA: fgets (iofgets.c:53)
==8197==    by 0x10BC64: main (ossec-regex.c:62)
==8197== 
==8197== HEAP SUMMARY:
==8197==     in use at exit: 100 bytes in 10 blocks
==8197==   total heap usage: 66 allocs, 56 frees, 2,614 bytes allocated
==8197== 
==8197== LEAK SUMMARY:
==8197==    definitely lost: 0 bytes in 0 blocks
==8197==    indirectly lost: 0 bytes in 0 blocks
==8197==      possibly lost: 0 bytes in 0 blocks
==8197==    still reachable: 100 bytes in 10 blocks
==8197==         suppressed: 0 bytes in 0 blocks
==8197== Rerun with --leak-check=full to see details of leaked memory
==8197== 
==8197== For counts of detected and suppressed errors, rerun with: -v
==8197== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Functional test

➜  ossec-testing git:(3813-fix-os-regex-invalid-read) ✗ sudo python runtests.py 
- [ File = ./tests/postfix.ini ] ---------
..
- [ File = ./tests/vsftpd.ini ] ---------
....
- [ File = ./tests/cimserver.ini ] ---------
..
- [ File = ./tests/opensmtpd.ini ] ---------
.......
- [ File = ./tests/apache.ini ] ---------
............
- [ File = ./tests/proftpd.ini ] ---------
.......
- [ File = ./tests/su.ini ] ---------
.....
- [ File = ./tests/web_appsec.ini ] ---------
...............................
- [ File = ./tests/sshd.ini ] ---------
...........................
- [ File = ./tests/exim.ini ] ---------
.....
- [ File = ./tests/rsh.ini ] ---------
..
- [ File = ./tests/doas.ini ] ---------
....
- [ File = ./tests/apparmor.ini ] ---------
.....
- [ File = ./tests/nginx.ini ] ---------
............
- [ File = ./tests/firewalld.ini ] ---------
..
- [ File = ./tests/syslog.ini ] ---------
.....
- [ File = ./tests/named.ini ] ---------
.....
- [ File = ./tests/samba.ini ] ---------
....
- [ File = ./tests/ossec.ini ] ---------
.....
- [ File = ./tests/sysmon.ini ] ---------
...
- [ File = ./tests/dovecot.ini ] ---------
...............
- [ File = ./tests/mailscanner.ini ] ---------
.
- [ File = ./tests/web_rules.ini ] ---------
.....
- [ File = ./tests/unbound.ini ] ---------

- [ File = ./tests/oscap.ini ] ---------
................................
- [ File = ./tests/cpanel.ini ] ---------
.......
- [ File = ./tests/sudo.ini ] ---------
........
- [ File = ./tests/modsecurity.ini ] ---------
......
- [ File = ./tests/pam.ini ] ---------
.....
- [ File = ./tests/systemd.ini ] ---------
.
- [ File = ./tests/netscreen.ini ] ---------
....
- [ File = ./tests/cisco_ios.ini ] ---------
.....

• Memory tests for Linux
  • Scan-build report
  • Coverity
  • Valgrind (memcheck and descriptor leaks check)
  • AddressSanitizer