Fix buffer overflow on empty strings in key.

[ealekseev]

Sometimes apache segfalult on memory copying when key.dptr is some
kind of empty string and key.dsize seems to be 0.

Investigating sefaults in our apache installation via gdb I get this:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:116
116    ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or directory.
(gdb) up
#1  0x00007fd26129cd58 in memcpy (__len=18446744073709551615, __src=0xcd7c68, _dest= <optimized out>) at /usr/include/bits/string3.h:51
51      return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
(gdb) up
#2  apr_pstrmemdup (a=<optimized out>, s=0xcd7c68 "", n=18446744073709551615) at strings/apr_strings.c:107
107    strings/apr_strings.c: No such file or directory.
(gdb) up
#3  0x00007fd253956987 in collections_remove_stale (msr=msr@entry=0xcc5b88, col_name=0xccd678 "ip") at persist_dbm.c:629
629    persist_dbm.c: No such file or directory.
(gdb) p s
$1 = <optimized out>
(gdb) down
#2  apr_pstrmemdup (a=<optimized out>, s=0xcd7c68 "", n=18446744073709551615) at strings/apr_strings.c:107
107    strings/apr_strings.c: No such file or directory.
(gdb) p s
$2 = 0xcd7c68 "" 
(gdb) p a
$3 = <optimized out>
(gdb) up
#3  0x00007fd253956987 in collections_remove_stale (msr=msr@entry=0xcc5b88, col_name=0xccd678 "ip") at persist_dbm.c:629
629    persist_dbm.c: No such file or directory.
(gdb) up
#4  0x00007fd25394393b in modsecurity_persist_data (msr=0xcc5b88) at modsecurity.c:232
232    modsecurity.c: No such file or directory.
(gdb) up
#5  modsecurity_process_phase_logging (msr=0xcc5b88) at modsecurity.c:640
640    in modsecurity.c
(gdb) up
#6  modsecurity_process_phase (msr=msr@entry=0xcc5b88, phase=phase@entry=5) at modsecurity.c:807
807    in modsecurity.c
(gdb) up
#7  0x00007fd253941cd2 in hook_log_transaction (r=<optimized out>) at mod_security2.c:1239
1239    mod_security2.c: No such file or directory.
(gdb) up
#8  0x000000000042cdd0 in ap_run_log_transaction (r=0xcc1be0) at protocol.c:1748
1748    protocol.c: No such file or directory.
(gdb) up
#9  0x000000000042d8c8 in ap_read_request (conn=conn@entry=0xcbbb00) at protocol.c:1078
1078    in protocol.c

Problem is here:
ModSecurity/apache2/persist_dbm.c:

rc = apr_sdbm_firstkey(dbm, &key);
while(rc == APR_SUCCESS) {
    char *s = apr_pstrmemdup(msr->mp, key.dptr, key.dsize - 1);  // <---- buffer overflow here
    *(char **)apr_array_push(keys_arr) = s;
    rc = apr_sdbm_nextkey(dbm, &key);
}

key.dptr = ""

key.dsize - 1 = 18446744073709551615 = 0xFFFFFFFFFFFFFFFF
key.dsize = 0

[marcstern]

Optimisation: "key.dsize ? (key.dsize - 1) : 0" instead of "strlen(key.dptr)"
Even better, I suspect that we shouldn't even perform the apr_array_push() at all:
if (key.dsize) {
char s = apr_pstrmemdup(msr->mp, key.dptr, key.dsize - 1);
*(char *)apr_array_push(keys_arr) = s;
}

[zimmerle]

Hi @ealekseev,

Thank you for the patch. I have also added the suggesting made by @marcstern. Code being cooked by our buildbots.

[ealekseev]

You are welcome!

[jpelaez01]

Thanks @ealekseev for the solution!

[zimmerle]

Hi @ealekseev,

Thank you for the patch again. Your code has been merged into your mainline. It will be released as part of 2.9.1.