Skip to content

Commit

Permalink
Fix invalid memory access (ntop#1596)
Browse files Browse the repository at this point in the history
We can access `flow->protos` union only after checking the protocol.

Checking `flow->detected_protocol.master_protocol` is redundant because
we already check it in `is_ndpi_proto`

```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==29739==ERROR: AddressSanitizer: SEGV on unknown address 0x000000353820 (pc 0x7f9b64dd2717 bp 0x7fff161a52f0 sp 0x7fff161a4aa8 T0)
==29739==The signal is caused by a READ memory access.
    #0 0x7f9b64dd2717  /build/glibc-SzIz7B/glibc-2.31/string/../sysdeps/x86_64/multiarch/strlen-avx2.S:96
    #1 0x555c65e597d8 in __interceptor_strlen (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader_with_main+0x6407d8) (BuildId: 11ac8ec30f1d49fb0276c9b03368e491505d2bba)
    #2 0x555c65fd85fa in ndpi_strdup /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:269:13
    #3 0x555c65f3e8c6 in process_ndpi_collected_info /home/ivan/svnrepos/nDPI/example/reader_util.c:1188:36
    #4 0x555c65f52cab in packet_processing /home/ivan/svnrepos/nDPI/example/reader_util.c:1567:2
    #5 0x555c65f4b632 in ndpi_workflow_process_packet /home/ivan/svnrepos/nDPI/example/reader_util.c:2110:10
    #6 0x555c65f04d29 in LLVMFuzzerTestOneInput /home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader.c:109:7
    #7 0x555c65f054bb in main /home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader.c:181:17
    ntop#8 0x7f9b64c6e082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    ntop#9 0x555c65e4253d in _start (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader_with_main+0x62953d) (BuildId: 11ac8ec30f1d49fb0276c9b03368e491505d2bba)

```

Found by oss-fuzzer.
See: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=48020
  • Loading branch information
IvanNardi authored Jun 14, 2022
1 parent 8dcaa5c commit bdf54d7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
8 changes: 5 additions & 3 deletions example/reader_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1157,10 +1157,12 @@ void process_ndpi_collected_info(struct ndpi_workflow * workflow, struct ndpi_fl
flow->ndpi_flow->protos.ssh.hassh_server);
}
/* TLS */
else if((is_ndpi_proto(flow, NDPI_PROTOCOL_TLS))
else if(is_ndpi_proto(flow, NDPI_PROTOCOL_TLS)
|| is_ndpi_proto(flow, NDPI_PROTOCOL_DTLS)
|| is_ndpi_proto(flow, NDPI_PROTOCOL_MAIL_SMTPS)
|| is_ndpi_proto(flow, NDPI_PROTOCOL_MAIL_IMAPS)
|| is_ndpi_proto(flow, NDPI_PROTOCOL_MAIL_POPS)
|| ((is_quic = is_ndpi_proto(flow, NDPI_PROTOCOL_QUIC)))
|| (flow->detected_protocol.master_protocol == NDPI_PROTOCOL_TLS)
|| (flow->ndpi_flow->protos.tls_quic.ja3_client[0] != '\0')
) {
flow->ssh_tls.ssl_version = flow->ndpi_flow->protos.tls_quic.ssl_version;

Expand Down
2 changes: 1 addition & 1 deletion src/include/ndpi_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ struct ndpi_flow_struct {
char *esni;
} encrypted_sni;
ndpi_cipher_weakness server_unsafe_cipher;
} tls_quic;
} tls_quic; /* Used also by DTLS and POPS/IMAPS/SMTPS */

struct {
char client_signature[48], server_signature[48];
Expand Down

0 comments on commit bdf54d7

Please sign in to comment.