From a55971501041a4b3eaf695fd84f8c29cf4a09758 Mon Sep 17 00:00:00 2001 From: Nardi Ivan Date: Thu, 4 Aug 2022 17:31:22 +0200 Subject: [PATCH] SoftEther: fix two heap-buffer-overflows The first change is a proper (hopefully) fix for the bug reported in 8b6a00f8. The second one is related to: ``` ==15096==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000d7a00 at pc 0x55a2c593bd0b bp 0x7ffc92021cd0 sp 0x7ffc92021478 READ of size 3 at 0x60f0000d7a00 thread T0 #0 0x55a2c593bd0a in strncmp (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x56fd0a) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #1 0x55a2c5d1d9f9 in dissect_softether_host_fqdn /home/ivan/svnrepos/nDPI/src/lib/protocols/softether.c:249:9 #2 0x55a2c5d1b55b in ndpi_search_softether /home/ivan/svnrepos/nDPI/src/lib/protocols/softether.c:348:9 #3 0x55a2c5b0e9c5 in check_ndpi_detection_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5407:6 #4 0x55a2c5b0f78b in check_ndpi_udp_flow_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5443:10 #5 0x55a2c5b0f12c in ndpi_check_flow_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5476:12 #6 0x55a2c5b20f39 in ndpi_detection_process_packet /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:6303:15 #7 0x55a2c5a3014c in packet_processing /home/ivan/svnrepos/nDPI/example/reader_util.c:1600:31 #8 0x55a2c5a29062 in ndpi_workflow_process_packet /home/ivan/svnrepos/nDPI/example/reader_util.c:2170:10 #9 0x55a2c59e51a2 in LLVMFuzzerTestOneInput /home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader.c:107:7 #10 0x55a2c590acb2 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x53ecb2) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #11 0x55a2c590a3c5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x53e3c5) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #12 0x55a2c590c0f6 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector>&) (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x5400f6) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #13 0x55a2c590c663 in fuzzer::Fuzzer::Loop(std::vector>&) (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x540663) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #14 0x55a2c58faff2 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x52eff2) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #15 0x55a2c5923c82 in main (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x557c82) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) #16 0x7f504ab98082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16 #17 0x55a2c58efb1d in _start (/home/ivan/svnrepos/nDPI/fuzz/fuzz_ndpi_reader+0x523b1d) (BuildId: ee8631c0950a8cded5ba60c17f09709bbebbe5d8) 0x60f0000d7a00 is located 0 bytes to the right of 176-byte region [0x60f0000d7950,0x60f0000d7a00) ``` Found by oss-fuzzer See: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49736 --- src/lib/protocols/softether.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/lib/protocols/softether.c b/src/lib/protocols/softether.c index 6afdebb4abd..377d9ed3aad 100644 --- a/src/lib/protocols/softether.c +++ b/src/lib/protocols/softether.c @@ -104,6 +104,10 @@ static size_t dissect_softether_type(enum softether_value_type t, } if (t == VALUE_DATA) { + if (siz == 0) + { + return 0; + } siz--; } v->value_size = siz; @@ -288,11 +292,6 @@ static int dissect_softether_ip_port(struct ndpi_flow_struct *flow, return 1; } - if (ip_port_separator < (char const *)packet->payload + NDPI_STATICSTRING_LEN("IP=")) - { - return 1; - } - ip_len = ndpi_min(sizeof(flow->protos.softether.ip) - 1, ip_port_separator - (char const *)packet->payload - NDPI_STATICSTRING_LEN("IP=")); @@ -300,15 +299,15 @@ static int dissect_softether_ip_port(struct ndpi_flow_struct *flow, ip_len); flow->protos.softether.ip[ip_len] = '\0'; - if (ip_port_separator < (char const *)packet->payload + - NDPI_STATICSTRING_LEN("IP=") + NDPI_STATICSTRING_LEN(",PORT=")) + if (packet->payload_packet_len < (ip_port_separator - (char const *)packet->payload) + + NDPI_STATICSTRING_LEN(",PORT=")) { return 1; } port_len = ndpi_min(sizeof(flow->protos.softether.port) - 1, - ip_port_separator - (char const *)packet->payload - - NDPI_STATICSTRING_LEN("IP=") - NDPI_STATICSTRING_LEN(",PORT=")); + packet->payload_packet_len - (ip_port_separator - (char const *)packet->payload) - + NDPI_STATICSTRING_LEN(",PORT=")); strncpy(flow->protos.softether.port, ip_port_separator + NDPI_STATICSTRING_LEN(",PORT="), port_len); flow->protos.softether.port[port_len] = '\0';