Skip to content

ahocorasick: fix mem leaked AC_NODE_T object (#2258)#2522

Merged
IvanNardi merged 1 commit into
ntop:devfrom
mmaatuq:ahocorasick_mem_leak_fix
Aug 23, 2024
Merged

ahocorasick: fix mem leaked AC_NODE_T object (#2258)#2522
IvanNardi merged 1 commit into
ntop:devfrom
mmaatuq:ahocorasick_mem_leak_fix

Conversation

@mmaatuq

@mmaatuq mmaatuq commented Aug 19, 2024

Copy link
Copy Markdown
Contributor

Please sign (check) the below before submitting the Pull Request:

Link to the related issue:

Describe changes:

skipping node at depth = AC_PATTRN_MAX_LENGTH inside ac_automata_walk() caused this leak, as one of the added patterns has len = AC_PATTRN_MAX_LENGTH (not including the null char), this change avoid this
Fixes:
==3162838==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:
0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
3 0x68a0fd in ndpi_calloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:67:13
4 0x9c5b3a in node_create /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:802:25
5 0x9c760e in node_create_next /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1007:10
6 0x9c6653 in ac_automata_add /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:255:19
7 0x5861d0 in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4573:8
8 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
9 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
10 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
11 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (Build
Id: 36867732af52eb22433f5e34fe5422a8af281aa6)
12 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 3
6867732af52eb22433f5e34fe5422a8af281aa6)
13 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
14 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
15 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

Indirect leak of 328 byte(s) in 1 object(s) allocated from:
0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
3 0x68a0fd in ndpi_calloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:67:13
4 0x9da348 in node_resize_mp /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1027:13
5 0x9c79d7 in node_register_matchstr /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1055:30
6 0x9c6f21 in ac_automata_add /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:280:6
7 0x5861d0 in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4573:8
8 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
9 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
10 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
11 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (Build
Id: 36867732af52eb22433f5e34fe5422a8af281aa6)
12 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 3
6867732af52eb22433f5e34fe5422a8af281aa6)
13 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
14 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
15 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

Indirect leak of 257 byte(s) in 1 object(s) allocated from:
0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
3 0x68a390 in ndpi_strdup /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:113:13
4 0x585e2b in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4558:14
5 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
6 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
7 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
8 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (BuildI
d: 36867732af52eb22433f5e34fe5422a8af281aa6)
9 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 36
867732af52eb22433f5e34fe5422a8af281aa6)
10 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
11 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
12 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

SUMMARY: AddressSanitizer: 625 byte(s) leaked in 3 allocation(s).

skipping node at depth = AC_PATTRN_MAX_LENGTH inside    caused this leak
this change avoid this
Fix:
==3162838==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
    2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
    3 0x68a0fd in ndpi_calloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:67:13
    4 0x9c5b3a in node_create /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:802:25
    5 0x9c760e in node_create_next /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1007:10
    6 0x9c6653 in ac_automata_add /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:255:19
    7 0x5861d0 in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4573:8
    8 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
    9 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
    10 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
    11 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (Build
Id: 36867732af52eb22433f5e34fe5422a8af281aa6)
    12 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 3
6867732af52eb22433f5e34fe5422a8af281aa6)
    13 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    14 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    15 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

Indirect leak of 328 byte(s) in 1 object(s) allocated from:
    0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
    2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
    3 0x68a0fd in ndpi_calloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:67:13
    4 0x9da348 in node_resize_mp /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1027:13
    5 0x9c79d7 in node_register_matchstr /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:1055:30
    6 0x9c6f21 in ac_automata_add /root/workspace/ntop/nDPI/src/lib/third_party/src/ahocorasick.c:280:6
    7 0x5861d0 in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4573:8
    8 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
    9 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
    10 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
    11 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (Build
Id: 36867732af52eb22433f5e34fe5422a8af281aa6)
    12 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 3
6867732af52eb22433f5e34fe5422a8af281aa6)
    13 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    14 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    15 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

Indirect leak of 257 byte(s) in 1 object(s) allocated from:
    0 0x50f072 in malloc (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x50f072) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    1 0x552f20 in malloc_wrapper /root/workspace/ntop/nDPI/fuzz/fuzz_common_code.c:16:31
    2 0x68a0a5 in ndpi_malloc /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:60:25
    3 0x68a390 in ndpi_strdup /root/workspace/ntop/nDPI/src/lib/ndpi_memory.c:113:13
    4 0x585e2b in ndpi_add_host_risk_mask /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4558:14
    5 0x590c0e in ndpi_handle_rule /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:4655:11
    6 0x58ffd5 in load_protocols_file_fd /root/workspace/ntop/nDPI/src/lib/ndpi_main.c:5446:8
    7 0x552c60 in LLVMFuzzerTestOneInput /root/workspace/ntop/nDPI/fuzz/fuzz_filecfg_protocols.c:21:3
    8 0x44be94 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x44be94) (BuildI
d: 36867732af52eb22433f5e34fe5422a8af281aa6)
    9 0x4334f7 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4334f7) (BuildId: 36
867732af52eb22433f5e34fe5422a8af281aa6)
    10 0x4391e1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x4
391e1) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    11 0x467323 in main (/media/veracrypt1/work-space/ntop/nDPI/fuzz/fuzz_filecfg_protocols+0x467323) (BuildId: 36867732af52eb22433f5e34fe5422a8af281aa6)
    ntop#12 0x7fce386e97e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: b81415c1738806b536fb1599d7af2d15bf6a86b7)

SUMMARY: AddressSanitizer: 625 byte(s) leaked in 3 allocation(s).

Signed-off-by: mmaatuq <mahmoudmatook.mm@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@IvanNardi

Copy link
Copy Markdown
Collaborator

@pavlinux , @vel21ripn what do you think?

@vel21ripn

Copy link
Copy Markdown
Contributor

It's very good that you were able to find and correct the error.

@IvanNardi IvanNardi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the expertise to evaluate this change, but I trust @vel21ripn comment.
Let's see what happens in the next days on oss-fuzz...
@mmaatuq, thank you very much for working on that long-standing issue

@IvanNardi IvanNardi merged commit f03938a into ntop:dev Aug 23, 2024
@mmaatuq

mmaatuq commented Aug 23, 2024

Copy link
Copy Markdown
Contributor Author

I don't have the expertise to evaluate this change, but I trust @vel21ripn comment. Let's see what happens in the next days on oss-fuzz... @mmaatuq, thank you very much for working on that long-standing issue

Hopefully this is the right solution for it, as I was thinking of another one , patterns length that are added should be < AC_PATTRN_MAX_LENGTH without equal.

thank you too.

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

Successfully merging this pull request may close these issues.

3 participants