From 346e274a1b1d713aedfc341f7d1300009f9f0e1a Mon Sep 17 00:00:00 2001 From: Ivan Nardi <12729895+IvanNardi@users.noreply.github.com> Date: Fri, 5 Aug 2022 13:43:09 +0200 Subject: [PATCH] SoftEther: fix two heap-buffer-overflows (#1695) 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 | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/lib/protocols/softether.c b/src/lib/protocols/softether.c index 59ba80b0377..10db4be2a37 100644 --- a/src/lib/protocols/softether.c +++ b/src/lib/protocols/softether.c @@ -98,7 +98,7 @@ static size_t dissect_softether_type(enum softether_value_type t, v->value.ptr.raw = payload + 4; u_int32_t siz = ntohl(get_u_int32_t(payload, 0)); - if(payload_len < siz + 3) + if(siz == 0 || payload_len < siz + 3) return 0; if(t == VALUE_DATA) @@ -263,9 +263,6 @@ static int dissect_softether_ip_port(struct ndpi_flow_struct *flow, if(ip_port_separator == NULL) 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=")); @@ -275,16 +272,14 @@ 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=")) - return 1; + 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=")); - - strncpy(flow->protos.softether.port, - ip_port_separator + 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';