From e66d7216b8de3aa479db82486dcf64a63d8353ff Mon Sep 17 00:00:00 2001 From: Luca Deri Date: Wed, 4 Dec 2019 10:52:51 +0100 Subject: Removed header space with JSON serialization --- src/lib/ndpi_serializer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/lib/ndpi_serializer.c b/src/lib/ndpi_serializer.c index baacfd403..36ff154d2 100644 --- a/src/lib/ndpi_serializer.c +++ b/src/lib/ndpi_serializer.c @@ -200,7 +200,7 @@ char* ndpi_serializer_get_buffer(ndpi_serializer *_serializer, u_int32_t *buffer *buffer_len = serializer->status.size_used; if(serializer->fmt == ndpi_serialization_format_json) { - while(buf[0] == '\0') + while((buf[0] == '\0') || (buf[0] == ' ')) buf++, *buffer_len = *buffer_len - 1; } -- cgit v1.2.3 From d62526f9ed0d504c928a3861b4838f7029bc0632 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Wed, 4 Dec 2019 13:06:05 +0100 Subject: Fix invalid reads and add valgrind test --- src/lib/protocols/http.c | 2 +- src/lib/protocols/ssh.c | 2 +- tests/vagrind_test.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100755 tests/vagrind_test.sh (limited to 'src') diff --git a/src/lib/protocols/http.c b/src/lib/protocols/http.c index 4382879d0..70ca0c389 100644 --- a/src/lib/protocols/http.c +++ b/src/lib/protocols/http.c @@ -484,7 +484,7 @@ static u_int16_t http_request_url_offset(struct ndpi_detection_module_struct *nd packet->payload_packet_len); /* Check first char */ - if(!strchr(http_fs,packet->payload[0])) return 0; + if(!packet->payload_packet_len || !strchr(http_fs,packet->payload[0])) return 0; /** FIRST PAYLOAD PACKET FROM CLIENT **/ diff --git a/src/lib/protocols/ssh.c b/src/lib/protocols/ssh.c index 5bdf78959..068d2c345 100644 --- a/src/lib/protocols/ssh.c +++ b/src/lib/protocols/ssh.c @@ -296,7 +296,7 @@ static void ndpi_search_ssh_tcp(struct ndpi_detection_module_struct *ndpi_struct flow->l4.tcp.ssh_stage = 3; return; } - } else { + } else if(packet->payload_packet_len > 5) { u_int8_t msgcode = *(packet->payload + 5); ndpi_MD5_CTX ctx; diff --git a/tests/vagrind_test.sh b/tests/vagrind_test.sh new file mode 100755 index 000000000..aa04dab40 --- /dev/null +++ b/tests/vagrind_test.sh @@ -0,0 +1,33 @@ +#!/bin/sh + +READER="valgrind -q --leak-check=full ../example/ndpiReader -p ../example/protos.txt -c ../example/categories.txt" + +RC=0 +PCAPS=`cd pcap; /bin/ls *.pcap` + +check_results() { + for f in $PCAPS; do + CMD="$READER -q -i pcap/$f > /tmp/reader.out" + $CMD + NUM_DIFF=0 + + if [ -f /tmp/reader.out ]; then + NUM_DIFF=`wc -l /tmp/reader.out` + fi + + if [ $NUM_DIFF -eq 0 ]; then + printf "%-32s\tOK\n" "$f" + else + printf "%-32s\tERROR\n" "$f" + echo "$CMD" + cat /tmp/reader.out + RC=1 + fi + + /bin/rm -f /tmp/reader.out + done +} + +check_results + +exit $RC -- cgit v1.2.3 From 2b3ef7e7622c51641e9e88fa76442cda086982fd Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Wed, 4 Dec 2019 14:21:04 +0100 Subject: Fix invalid memory access --- src/lib/protocols/tls.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/lib/protocols/tls.c b/src/lib/protocols/tls.c index 23c47d7cd..cd729fc3c 100644 --- a/src/lib/protocols/tls.c +++ b/src/lib/protocols/tls.c @@ -308,7 +308,11 @@ int getTLScertificate(struct ndpi_detection_module_struct *ndpi_struct, #endif offset += 2 + 1; - extension_len = ntohs(*((u_int16_t*)&packet->payload[offset])); + + if(offset > packet->payload_packet_len) + extension_len = ntohs(*((u_int16_t*)&packet->payload[offset])); + else + extension_len = 0; #ifdef DEBUG_TLS printf("TLS [server][extension_len: %u]\n", extension_len); -- cgit v1.2.3 From f26096ee4cf730244b3db577259f84b0cd37e521 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Wed, 4 Dec 2019 15:20:49 +0100 Subject: Fix reversed offset check --- src/lib/protocols/tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/lib/protocols/tls.c b/src/lib/protocols/tls.c index cd729fc3c..2f4959b81 100644 --- a/src/lib/protocols/tls.c +++ b/src/lib/protocols/tls.c @@ -309,7 +309,7 @@ int getTLScertificate(struct ndpi_detection_module_struct *ndpi_struct, offset += 2 + 1; - if(offset > packet->payload_packet_len) + if((offset + 1) < packet->payload_packet_len) /* +1 because we are goint to read 2 bytes */ extension_len = ntohs(*((u_int16_t*)&packet->payload[offset])); else extension_len = 0; -- cgit v1.2.3 From 226a9abf2235fd9e87353ffe727babad71fb7274 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Wed, 4 Dec 2019 18:34:08 +0100 Subject: Additional memory bounds checks --- src/lib/ndpi_main.c | 4 +++- src/lib/protocols/irc.c | 2 +- src/lib/protocols/tls.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/lib/ndpi_main.c b/src/lib/ndpi_main.c index 40bf9ae20..18173d555 100644 --- a/src/lib/ndpi_main.c +++ b/src/lib/ndpi_main.c @@ -4252,6 +4252,8 @@ ndpi_protocol ndpi_detection_giveup(struct ndpi_detection_module_struct *ndpi_st if(flow->host_server_name[0] != '\0') { ndpi_protocol_match_result ret_match; + memset(&ret_match, 0, sizeof(ret_match)); + ndpi_match_host_subprotocol(ndpi_str, flow, (char *)flow->host_server_name, strlen((const char*)flow->host_server_name), @@ -5096,7 +5098,7 @@ void ndpi_parse_packet_line_info(struct ndpi_detection_module_struct *ndpi_str, for(a = 0; (a < packet->payload_packet_len) && (packet->parsed_lines < NDPI_MAX_PARSE_LINES_PER_PACKET); a++) { - if((a + 1) == packet->payload_packet_len) + if((a + 1) >= packet->payload_packet_len) return; /* Return if only one byte remains (prevent invalid reads past end-of-buffer) */ if(get_u_int16_t(packet->payload, a) == ntohs(0x0d0a)) { /* If end of line char sequence CR+NL "\r\n", process line */ diff --git a/src/lib/protocols/irc.c b/src/lib/protocols/irc.c index 5ae0e34f7..37cfbe1ed 100644 --- a/src/lib/protocols/irc.c +++ b/src/lib/protocols/irc.c @@ -495,7 +495,7 @@ void ndpi_search_irc_tcp(struct ndpi_detection_module_struct *ndpi_struct, struc packet->parsed_lines = 0; } for (i = 0; i < packet->parsed_lines; i++) { - if (packet->line[i].ptr[0] == ':') { + if ((packet->line[i].len > 0) && packet->line[i].ptr[0] == ':') { flow->l4.tcp.irc_3a_counter++; if (flow->l4.tcp.irc_3a_counter == 7) { /* ':' == 0x3a */ NDPI_LOG_INFO(ndpi_struct, "found irc. 0x3a. seven times."); diff --git a/src/lib/protocols/tls.c b/src/lib/protocols/tls.c index 2f4959b81..ed92814d9 100644 --- a/src/lib/protocols/tls.c +++ b/src/lib/protocols/tls.c @@ -874,7 +874,7 @@ int getSSCertificateFingerprint(struct ndpi_detection_module_struct *ndpi_struct return(0); /* That's all */ } else if(flow->l4.tcp.tls_seen_certificate) return(0); /* That's all */ - else if(packet->payload_packet_len > flow->l4.tcp.tls_record_offset+7) { + else if(packet->payload_packet_len > flow->l4.tcp.tls_record_offset+7+1/* +1 because we are going to read 2 bytes */) { /* This is a handshake but not a certificate record */ u_int16_t len = ntohs(*(u_int16_t*)&packet->payload[flow->l4.tcp.tls_record_offset+7]); -- cgit v1.2.3 From d37b69ce9c9caa979de7c511e33cb7d1cf5fbc91 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Wed, 4 Dec 2019 18:36:54 +0100 Subject: Fix corner case causing access to already freed memory --- src/lib/ndpi_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/lib/ndpi_main.c b/src/lib/ndpi_main.c index 18173d555..e31237980 100644 --- a/src/lib/ndpi_main.c +++ b/src/lib/ndpi_main.c @@ -4702,7 +4702,7 @@ ndpi_protocol ndpi_detection_process_packet(struct ndpi_detection_module_struct ndpi_process_extra_packet(ndpi_str, flow, packet, packetlen, current_tick_l, src, dst); /* Update in case of new match */ ret.master_protocol = flow->detected_protocol_stack[1], ret.app_protocol = flow->detected_protocol_stack[0], ret.category = flow->category;; - return(ret); + goto invalidate_ptr; } else goto ret_protocols; } -- cgit v1.2.3