From 1ad3de3fc8b3f5d2d077243775b49a1756253ae8 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Fri, 15 Jan 2021 11:51:02 +0100 Subject: [PATCH 01/22] Fix leak in associative array implementation --- common/lib/r_assoc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/r_assoc.c b/common/lib/r_assoc.c index 6a2393d..0e9e16e 100644 --- a/common/lib/r_assoc.c +++ b/common/lib/r_assoc.c @@ -121,6 +121,9 @@ int r_assoc_destroy(assocp) for(i=0;isize;i++) destroy_assoc_chain(assoc->chains[i]); + free(assoc->chains); + free(assoc); + return(0); } From 8ef5540e585264f47d9d6c4c45a43cd9899110e8 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Fri, 15 Jan 2021 11:59:26 +0100 Subject: [PATCH 02/22] Close everything properly in case of SIGINT --- base/pcap-snoop.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/base/pcap-snoop.c b/base/pcap-snoop.c index 804b93c..8524366 100644 --- a/base/pcap-snoop.c +++ b/base/pcap-snoop.c @@ -127,6 +127,9 @@ int print_version() } pcap_t *p; +proto_mod *mod=&ssl_mod; +n_handler *n; +char *interface_name=0; void sig_handler(int sig) { int freed_conn = 0; @@ -140,6 +143,9 @@ void sig_handler(int sig) if(p) pcap_close(p); + if(interface_name) + free(interface_name); + exit(0); } @@ -287,7 +293,6 @@ int main(argc,argv) char **argv; { int r; - n_handler *n; #ifdef _WIN32 __declspec(dllimport) char *optarg; __declspec(dllimport) int optind; @@ -296,10 +301,8 @@ int main(argc,argv) extern int optind; #endif pcap_if_t *interfaces; - char *interface_name=0; char *file=0; char *filter=0; - proto_mod *mod=&ssl_mod; bpf_u_int32 localnet,netmask; int c; module_def *m=0; From 96021582f46b7be258ce70cc7614e9d6034543a5 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Fri, 15 Jan 2021 12:11:37 +0100 Subject: [PATCH 03/22] Avoid leak by freeing SSL decoding context properly --- base/network.c | 9 +++++++-- base/network.h | 2 +- base/pcap-snoop.c | 3 +++ base/proto_mod.h | 1 + ssl/ssl_analyze.c | 11 +++++++++++ ssl/ssldecode.c | 19 +++++++++++++++++++ ssl/ssldecode.h | 1 + 7 files changed, 43 insertions(+), 3 deletions(-) diff --git a/base/network.c b/base/network.c index b44b8a2..766c169 100644 --- a/base/network.c +++ b/base/network.c @@ -86,17 +86,22 @@ int network_handler_create(mod,handlerp) _status=0; abort: if(_status){ - network_handler_destroy(&handler); + network_handler_destroy(mod, &handler); } return(_status); } -int network_handler_destroy(handlerp) +int network_handler_destroy(mod,handlerp) + proto_mod *mod; n_handler **handlerp; { + n_handler *handler=0; if(!handlerp || !*handlerp) return(0); + handler = *handlerp; + + mod->vtbl->destroy_ctx(mod->handle,&handler->ctx); free(*handlerp); *handlerp=0; return(0); diff --git a/base/network.h b/base/network.h index 2a21635..65cbb1c 100644 --- a/base/network.h +++ b/base/network.h @@ -75,7 +75,7 @@ typedef struct packet_ packet; int network_handler_create PROTO_LIST((proto_mod *mod, n_handler **handlerp)); -int network_handler_destroy PROTO_LIST((n_handler **handlerp)); +int network_handler_destroy PROTO_LIST((proto_mod *mod,n_handler **handlerp)); int network_process_packet PROTO_LIST((n_handler *handler, struct timeval *timestamp,UCHAR *data,int length)); int packet_copy PROTO_LIST((packet *in,packet **out)); diff --git a/base/pcap-snoop.c b/base/pcap-snoop.c index 8524366..bfa6161 100644 --- a/base/pcap-snoop.c +++ b/base/pcap-snoop.c @@ -141,6 +141,8 @@ void sig_handler(int sig) if(freed_conn && !(NET_print_flags & NET_PRINT_JSON)) printf("Cleaned %d remaining connection(s) from connection pool\n", freed_conn); + network_handler_destroy(mod, &n); + if(p) pcap_close(p); if(interface_name) @@ -496,6 +498,7 @@ int main(argc,argv) if(freed_conn && !(NET_print_flags & NET_PRINT_JSON)) printf("Cleaned %d remaining connection(s) from connection pool\n", freed_conn); + network_handler_destroy(mod, &n); pcap_close(p); free(n); diff --git a/base/proto_mod.h b/base/proto_mod.h index 8dddaaf..db5502a 100644 --- a/base/proto_mod.h +++ b/base/proto_mod.h @@ -62,6 +62,7 @@ struct proto_mod_vtbl_ { proto_obj **objp, struct in_addr *i_addr,u_short i_port, struct in_addr *r_addr,u_short r_port,struct timeval *time_base)); + int (*destroy_ctx) PROTO_LIST((void *handle,proto_ctx **ctxp)); int (*destroy) PROTO_LIST((proto_obj **objp)); int (*data) PROTO_LIST((proto_obj *obj,segment *data,int direction)); int (*close) PROTO_LIST((proto_obj *obj,packet *p,int direction)); diff --git a/ssl/ssl_analyze.c b/ssl/ssl_analyze.c index 2dda2ab..09ce8f2 100644 --- a/ssl/ssl_analyze.c +++ b/ssl/ssl_analyze.c @@ -61,6 +61,7 @@ static int create_ssl_analyzer PROTO_LIST((void *handle, proto_ctx *ctx,tcp_conn *conn,proto_obj **objp, struct in_addr *i_addr,u_short i_port, struct in_addr *r_addr,u_short r_port, struct timeval *base_time)); +static int destroy_ssl_ctx PROTO_LIST((void *handle,proto_ctx **ctxp)); static int destroy_ssl_analyzer PROTO_LIST((proto_obj **objp)); static int read_ssl_record PROTO_LIST((ssl_obj *obj,r_queue *q,segment *seg, int offset,segment **lastp,int *offsetp)); @@ -228,6 +229,15 @@ static int create_ssl_ctx(handle,ctxp) return(_status); } +static int destroy_ssl_ctx(handle,ctxp) + void *handle; + proto_ctx **ctxp; + { + ssl_decode_ctx *ctx=0; + ctx=(ssl_decode_ctx *) *ctxp; + ssl_decode_ctx_destroy(&ctx); + } + static int create_ssl_analyzer(void *handle, proto_ctx *ctx, tcp_conn *conn, proto_obj **objp, struct in_addr *i_addr, u_short i_port, struct in_addr *r_addr, u_short r_port, struct timeval *base_time) @@ -635,6 +645,7 @@ static struct proto_mod_vtbl_ ssl_vtbl ={ parse_ssl_flag, create_ssl_ctx, create_ssl_analyzer, + destroy_ssl_ctx, destroy_ssl_analyzer, data_ssl_analyzer, close_ssl_analyzer, diff --git a/ssl/ssldecode.c b/ssl/ssldecode.c index 14c9e2f..162a3f3 100644 --- a/ssl/ssldecode.c +++ b/ssl/ssldecode.c @@ -191,6 +191,25 @@ int ssl_decode_ctx_create(dp,keyfile,pass,keylogfile) #endif } +int ssl_decode_ctx_destroy(dp) + ssl_decode_ctx **dp; + { +#ifdef OPENSSL + ssl_decode_ctx *d = *dp; + if(d->ssl_key_log_file) { + fclose(d->ssl_key_log_file); + } + + r_assoc *x = d->session_cache; + r_assoc_destroy(&d->session_cache); + + SSL_CTX_free(d->ssl_ctx); + SSL_free(d->ssl); + free(d); +#endif + return(0); + } + int ssl_decoder_create(dp,ctx) ssl_decoder **dp; ssl_decode_ctx *ctx; diff --git a/ssl/ssldecode.h b/ssl/ssldecode.h index 74b8a6b..a878716 100644 --- a/ssl/ssldecode.h +++ b/ssl/ssldecode.h @@ -52,6 +52,7 @@ int ssl_decode_ctx_create PROTO_LIST((ssl_decode_ctx **ctx, char *keyfile,char *password,char *keylogfile)); +int ssl_decode_ctx_destroy(ssl_decode_ctx **dp); int ssl_decoder_destroy PROTO_LIST((ssl_decoder **dp)); int ssl_decoder_create PROTO_LIST((ssl_decoder **dp,ssl_decode_ctx *ctx)); int ssl_set_client_random PROTO_LIST((ssl_decoder *dp, From afedb20a974d73d47c9e1aefc6c155a47e67cbde Mon Sep 17 00:00:00 2001 From: William Robinet Date: Tue, 19 Jan 2021 16:32:55 +0100 Subject: [PATCH 04/22] Add proper return value --- ssl/ssl_analyze.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ssl/ssl_analyze.c b/ssl/ssl_analyze.c index 09ce8f2..7184736 100644 --- a/ssl/ssl_analyze.c +++ b/ssl/ssl_analyze.c @@ -236,6 +236,7 @@ static int destroy_ssl_ctx(handle,ctxp) ssl_decode_ctx *ctx=0; ctx=(ssl_decode_ctx *) *ctxp; ssl_decode_ctx_destroy(&ctx); + return 0; } static int create_ssl_analyzer(void *handle, proto_ctx *ctx, tcp_conn *conn, From fba06b5c791a9635eecf5ad6870959522829f243 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Tue, 19 Jan 2021 17:25:02 +0100 Subject: [PATCH 05/22] Output error to stderr --- base/tcppack.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/base/tcppack.c b/base/tcppack.c index 330d366..f44777f 100644 --- a/base/tcppack.c +++ b/base/tcppack.c @@ -241,8 +241,7 @@ static int process_data_segment(conn,handler,p,stream,direction) l=p->len - p->tcp->th_off * 4; if(l < 0) { - if(!(NET_print_flags & NET_PRINT_JSON)) - printf("Malformed packet, computed TCP segment size is negative, skipping ...\n"); + fprintf(stderr,"Malformed packet, computed TCP segment size is negative, skipping ...\n"); return(0); } From 31464b75b008a89ae987264caf0b7f9d97f03763 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Tue, 19 Jan 2021 17:27:46 +0100 Subject: [PATCH 06/22] Decode ClientHello v2 properly --- ssl/sslprint.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/ssl/sslprint.c b/ssl/sslprint.c index afbfa43..21cb378 100644 --- a/ssl/sslprint.c +++ b/ssl/sslprint.c @@ -95,6 +95,7 @@ int process_v2_hello(ssl,seg) { int r; int rec_len; + int _status; UINT4 cs_len; UINT4 sid_len; UINT4 chall_len; @@ -104,36 +105,39 @@ int process_v2_hello(ssl,seg) UCHAR random[32]; if(seg->len==0) - return(SSL_NO_DATA); - + ABORT(SSL_NO_DATA); + d.data=seg->data; d.len=seg->len; /* First check the message length. */ if(d.len<4) - return(SSL_BAD_CONTENT_TYPE); + ABORT(SSL_BAD_CONTENT_TYPE); rec_len=((d.data[0] & 0x7f)<<8) | (d.data[1]); d.data+=2; d.len-=2; if(d.len!=rec_len) /* Whatever this is it isn't valid SSLv2*/ - return(SSL_BAD_CONTENT_TYPE); + ABORT(SSL_BAD_CONTENT_TYPE); /* If msg_type==1 then we've got a v2 message (or trash)*/ if(*d.data++!=1) - return(SSL_BAD_CONTENT_TYPE); + ABORT(SSL_BAD_CONTENT_TYPE); d.len--; SSL_DECODE_UINT16(ssl,"Version number",P_DC,&d,&ver); /* We can't handle real v2 clients*/ if(ver<=2){ explain(ssl,"Version 2 Client.\n"); - return(SSL_BAD_DATA); + ABORT(SSL_BAD_DATA); } + ssl->cur_json_st = json_object_new_object(); ssl_print_record_num(ssl); ssl_print_timestamp(ssl,&seg->p->ts); ssl_print_direction_indicator(ssl,DIR_I2R); explain(ssl," SSLv2 compatible client hello\n"); + json_object_object_add(ssl->cur_json_st, "msg_type", json_object_new_string("Handshake")); + json_object_object_add(ssl->cur_json_st, "handshake_type", json_object_new_string("ClientHello_v2_compat")); INDENT_INCR; @@ -148,7 +152,7 @@ int process_v2_hello(ssl,seg) if(cs_len%3){ fprintf(stderr,"Bad cipher spec length %d\n",cs_len); - return(SSL_BAD_DATA); + ABORT(SSL_BAD_DATA); } P_(P_HL){ explain(ssl,"cipher suites\n"); @@ -166,12 +170,12 @@ int process_v2_hello(ssl,seg) if(sid_len!=0){ fprintf(stderr,"Session ID field should be zero length\n"); - return(SSL_BAD_DATA); + ABORT(SSL_BAD_DATA); } if(chall_len<16 || chall_len>32){ fprintf(stderr,"Invalid challenge length %d\n",chall_len); - return(SSL_BAD_DATA); + ABORT(SSL_BAD_DATA); } SSL_DECODE_OPAQUE_ARRAY(ssl,0,chall_len, @@ -195,7 +199,18 @@ int process_v2_hello(ssl,seg) } INDENT_POP; - return(0); + + _status=0; + + abort: + if(ssl->cur_json_st) { + if(SSL_print_flags & SSL_PRINT_JSON) + printf("%s\n", json_object_to_json_string(ssl->cur_json_st)); + json_object_put(ssl->cur_json_st); + ssl->cur_json_st = NULL; + } + + return(_status); } int ssl_decode_switch(ssl,dtable,value,dir,seg,data) From 47d80c79bd46cbec5868c7469a95c32bf7209534 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Tue, 19 Jan 2021 17:29:04 +0100 Subject: [PATCH 07/22] Bump version to 1.3 in configure.ac --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2101fdb..9321d8f 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.69]) -AC_INIT([ssldump], [1.1]) +AC_INIT([ssldump], [1.3]) AM_INIT_AUTOMAKE([subdir-objects]) AC_CONFIG_SRCDIR([base/pcap-snoop.c]) AC_CONFIG_HEADERS([config.h]) From 2a98fb08fe748fb76f688463a8ea634d04502338 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 20 Jan 2021 10:34:38 +0100 Subject: [PATCH 08/22] Limit length during server name decoding --- ssl/ssl.enums.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ssl/ssl.enums.c b/ssl/ssl.enums.c index 4a76035..f3f4dea 100644 --- a/ssl/ssl.enums.c +++ b/ssl/ssl.enums.c @@ -2969,6 +2969,7 @@ static int decode_server_name_type_host_name(ssl,dir,seg,data) if (server_name != NULL) { if (ssl->server_name) free(ssl->server_name); + if (l > data->len) l = data->len; memcpy(server_name,data->data,l); ssl->server_name = server_name; } From 10963dd98169a866d3ff827b41db9a91a339a061 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 20 Jan 2021 10:47:52 +0100 Subject: [PATCH 09/22] Check return code after string extraction --- ssl/sslprint.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ssl/sslprint.c b/ssl/sslprint.c index 21cb378..3864773 100644 --- a/ssl/sslprint.c +++ b/ssl/sslprint.c @@ -291,7 +291,10 @@ int ssl_expand_record(ssl,q,direction,data,len) printf(" unknown record type: %d\n", ct); ERETURN(r); } - ssl_get_enum_str(ssl,enumstr,ContentType_decoder,ct); + if((r=ssl_get_enum_str(ssl,enumstr,ContentType_decoder,ct))) { + strncpy(enumstr, "Unknown", 20); + } + json_object_object_add(jobj, "msg_type", json_object_new_string(enumstr)); if(!(SSL_print_flags & SSL_PRINT_JSON)) From d1f8d01d4b8dd83085918cc95c7eafa57a85425f Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 20 Jan 2021 11:22:55 +0100 Subject: [PATCH 10/22] Check packet size before looking at IP header --- base/network.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/base/network.c b/base/network.c index 766c169..fbbbbf0 100644 --- a/base/network.c +++ b/base/network.c @@ -126,6 +126,12 @@ int network_process_packet(handler,timestamp,data,length) p.len=length; p.ip=(struct ip *)data; + if(p.len < 20) { + if(!(NET_print_flags & NET_PRINT_JSON)) + printf("Malformed packet, packet too small to contain IP header, skipping ...\n"); + return(0); + } + /*Handle, or rather mishandle, fragmentation*/ off=ntohs(p.ip->ip_off); From b3316bb5fdd44a5e455e5a3d386c291c966fc630 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Thu, 21 Jan 2021 09:57:27 +0100 Subject: [PATCH 11/22] Fix for crash if length of captured frame is less than Ethernet header size --- base/pcap-snoop.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/base/pcap-snoop.c b/base/pcap-snoop.c index bfa6161..5901e8b 100644 --- a/base/pcap-snoop.c +++ b/base/pcap-snoop.c @@ -176,6 +176,12 @@ void pcap_cb(ptr,hdr,data) len-=4; break; case DLT_EN10MB: + if(len < sizeof(struct ether_header)) { + if(!(NET_print_flags & NET_PRINT_JSON)) + printf("Frame size too small to contain Ethernet header, skipping ...\n"); + return; + } + type=ntohs(e_hdr->ether_type); data+=sizeof(struct ether_header); From 2a7b0f664f509903883c9c2dfcbb8c7457504007 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:30:28 +0100 Subject: [PATCH 12/22] Cleanup before exit on error --- base/pcap-snoop.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/base/pcap-snoop.c b/base/pcap-snoop.c index 5901e8b..7697f47 100644 --- a/base/pcap-snoop.c +++ b/base/pcap-snoop.c @@ -104,6 +104,7 @@ int err_exit(str,num) int num; { fprintf(stderr,"ERROR: %s\n",str); + sig_handler(SIGQUIT); exit(num); } @@ -130,6 +131,8 @@ pcap_t *p; proto_mod *mod=&ssl_mod; n_handler *n; char *interface_name=0; +char *file=0; +char *filter=0; void sig_handler(int sig) { int freed_conn = 0; @@ -147,8 +150,12 @@ void sig_handler(int sig) pcap_close(p); if(interface_name) free(interface_name); + if(filter) + free(filter); + if(file) + free(file); - exit(0); + exit(sig); } void pcap_cb(ptr,hdr,data) @@ -309,8 +316,6 @@ int main(argc,argv) extern int optind; #endif pcap_if_t *interfaces; - char *file=0; - char *filter=0; bpf_u_int32 localnet,netmask; int c; module_def *m=0; From f24eafd8b4ec085e37388c80f9916f7590a7f472 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:31:28 +0100 Subject: [PATCH 13/22] Check timestamp_diff return code correctly --- base/tcpconn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/tcpconn.c b/base/tcpconn.c index cee0e9f..820f50c 100644 --- a/base/tcpconn.c +++ b/base/tcpconn.c @@ -170,7 +170,8 @@ int clean_old_conn() { while(conn) { tcpconn = &conn->conn; conn=conn->next; - timestamp_diff(&last_packet_seen_time, &tcpconn->last_seen_time, &dt); + if(timestamp_diff(&last_packet_seen_time, &tcpconn->last_seen_time, &dt)) + continue; if(dt.tv_sec > conn_ttl) { i++; tcp_destroy_conn(tcpconn); From 4997301a9d667dcdf06103de625159ad4a5c1593 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:32:18 +0100 Subject: [PATCH 14/22] Exit process_tcp_packet() in case TCP header is incomplete --- base/tcppack.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/base/tcppack.c b/base/tcppack.c index f44777f..1680e79 100644 --- a/base/tcppack.c +++ b/base/tcppack.c @@ -80,7 +80,10 @@ int process_tcp_packet(handler,ctx,p) int direction; stream_data *stream; tcp_conn *conn; - + + if(p->len < 20) + ABORT(1); + p->tcp=(struct tcphdr *)p->data; print_tcp_packet(p); From 59b88f4e2b4076d17ed9c3a8fa18d07d9a7b3a7a Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:32:57 +0100 Subject: [PATCH 15/22] Check ssl_decode_enum() return code correctly --- ssl/ssl.enums.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ssl/ssl.enums.c b/ssl/ssl.enums.c index f3f4dea..d441fef 100644 --- a/ssl/ssl.enums.c +++ b/ssl/ssl.enums.c @@ -238,8 +238,9 @@ static int decode_HandshakeType_ClientHello(ssl,dir,seg,data) } for(;cslen;cslen-=2){ - ssl_decode_enum(ssl,0,2,cipher_suite_decoder, - 0,data,&cs); + if(ssl_decode_enum(ssl,0,2,cipher_suite_decoder, + 0,data,&cs)) + return(1); ssl_print_cipher_suite(ssl,(vj<<8)|vn,P_HL,cs); LF; } From 673f7ce85f8bd86b2149ff241bc2566b4f929046 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:35:27 +0100 Subject: [PATCH 16/22] Avoid client_random related leak --- ssl/ssldecode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ssl/ssldecode.c b/ssl/ssldecode.c index 162a3f3..7ecd3cf 100644 --- a/ssl/ssldecode.c +++ b/ssl/ssldecode.c @@ -268,6 +268,7 @@ int ssl_set_client_random(d,msg,len) #ifdef OPENSSL int r; + r_data_destroy(&d->client_random); if((r=r_data_create(&d->client_random,msg,len))) ERETURN(r); #endif From ee1d6de1f875174d8b3ca0b379f8e82ce93bc68b Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:36:56 +0100 Subject: [PATCH 17/22] Avoid server_random related leak --- ssl/ssldecode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ssl/ssldecode.c b/ssl/ssldecode.c index 7ecd3cf..9a4edde 100644 --- a/ssl/ssldecode.c +++ b/ssl/ssldecode.c @@ -282,7 +282,8 @@ int ssl_set_server_random(d,msg,len) { #ifdef OPENSSL int r; - + + r_data_destroy(&d->server_random); if((r=r_data_create(&d->server_random,msg,len))) ERETURN(r); #endif From cc1e752167adba315e0186eb3505dc225e4daee1 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:37:29 +0100 Subject: [PATCH 18/22] Avoid client session_id related leak --- ssl/ssldecode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ssl/ssldecode.c b/ssl/ssldecode.c index 9a4edde..b44128d 100644 --- a/ssl/ssldecode.c +++ b/ssl/ssldecode.c @@ -298,9 +298,11 @@ int ssl_set_client_session_id(d,msg,len) #ifdef OPENSSL int r; - if(len>0) + if(len>0) { + r_data_destroy(&d->session_id); if((r=r_data_create(&d->session_id,msg,len))) ERETURN(r); + } #endif return(0); } From 2d3d5d80459424f7832b4e1c7ececc68ed5cc3e1 Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:38:13 +0100 Subject: [PATCH 19/22] Abort properly on decode error --- ssl/sslprint.c | 12 ++++++------ ssl/sslprint.h | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ssl/sslprint.c b/ssl/sslprint.c index 3864773..06ec409 100644 --- a/ssl/sslprint.c +++ b/ssl/sslprint.c @@ -124,7 +124,7 @@ int process_v2_hello(ssl,seg) ABORT(SSL_BAD_CONTENT_TYPE); d.len--; - SSL_DECODE_UINT16(ssl,"Version number",P_DC,&d,&ver); + SSL_DECODE_UINT16_ABORT(ssl,"Version number",P_DC,&d,&ver); /* We can't handle real v2 clients*/ if(ver<=2){ explain(ssl,"Version 2 Client.\n"); @@ -146,9 +146,9 @@ int process_v2_hello(ssl,seg) ver&0xff); LF; } - SSL_DECODE_UINT16(ssl,"cipher_spec_length",P_DC,&d,&cs_len); - SSL_DECODE_UINT16(ssl,"session_id_length",P_DC,&d,&sid_len); - SSL_DECODE_UINT16(ssl,"challenge_length",P_DC,&d,&chall_len); + SSL_DECODE_UINT16_ABORT(ssl,"cipher_spec_length",P_DC,&d,&cs_len); + SSL_DECODE_UINT16_ABORT(ssl,"session_id_length",P_DC,&d,&sid_len); + SSL_DECODE_UINT16_ABORT(ssl,"challenge_length",P_DC,&d,&chall_len); if(cs_len%3){ fprintf(stderr,"Bad cipher spec length %d\n",cs_len); @@ -161,7 +161,7 @@ int process_v2_hello(ssl,seg) for(;cs_len;cs_len-=3){ UINT4 val; - SSL_DECODE_UINT24(ssl,0,0,&d,&val); + SSL_DECODE_UINT24_ABORT(ssl,0,0,&d,&val); ssl_print_cipher_suite(ssl,ver,P_HL,val); P_(P_HL){ explain(ssl,"\n"); @@ -178,7 +178,7 @@ int process_v2_hello(ssl,seg) ABORT(SSL_BAD_DATA); } - SSL_DECODE_OPAQUE_ARRAY(ssl,0,chall_len, + SSL_DECODE_OPAQUE_ARRAY_ABORT(ssl,0,chall_len, 0,&d,&chall); P_(P_DC){ exdump(ssl,"Challenge",&chall); diff --git a/ssl/sslprint.h b/ssl/sslprint.h index 2e6f737..e180abe 100644 --- a/ssl/sslprint.h +++ b/ssl/sslprint.h @@ -88,6 +88,12 @@ int exstr PROTO_LIST((ssl_obj *ssl,char *name,Data *data)); #define SSL_DECODE_UINT32(a,n,b,c,d) if((r=ssl_decode_uintX(a,n,4,b,c,d))) ERETURN(r) #define SSL_DECODE_OPAQUE_ARRAY(a,n,b,c,d,e) if((r=ssl_decode_opaque_array(a,n,b,c,d,e))) ERETURN(r) #define SSL_DECODE_ENUM(a,b,c,d,e,f,g) if((r=ssl_decode_enum(a,b,c,d,e,f,g))) ERETURN(r) +#define SSL_DECODE_UINT8_ABORT(a,n,b,c,d) if((r=ssl_decode_uintX(a,n,1,b,c,d))) ABORT(r) +#define SSL_DECODE_UINT16_ABORT(a,n,b,c,d) if((r=ssl_decode_uintX(a,n,2,b,c,d))) ABORT(r) +#define SSL_DECODE_UINT24_ABORT(a,n,b,c,d) if((r=ssl_decode_uintX(a,n,3,b,c,d))) ABORT(r) +#define SSL_DECODE_UINT32_ABORT(a,n,b,c,d) if((r=ssl_decode_uintX(a,n,4,b,c,d))) ABORT(r) +#define SSL_DECODE_OPAQUE_ARRAY_ABORT(a,n,b,c,d,e) if((r=ssl_decode_opaque_array(a,n,b,c,d,e))) ABORT(r) +#define SSL_DECODE_ENUM_ABORT(a,b,c,d,e,f,g) if((r=ssl_decode_enum(a,b,c,d,e,f,g))) ABORT(r) #define P_(p) if((p==SSL_PRINT_ALL) || (p & SSL_print_flags)) #define INDENT if(!(NET_print_flags & NET_PRINT_JSON)) do {int i; for(i=0;i<(ssl->indent_depth + ssl->indent_name_len);i++) printf("%s",SSL_print_flags & SSL_PRINT_NROFF?" ":" ");} while(0) From 64effa3bb93c3a219fb0afd868c5bc2609093ced Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:39:03 +0100 Subject: [PATCH 20/22] Clean remaining json object in case of error --- ssl/sslxprint.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ssl/sslxprint.c b/ssl/sslxprint.c index 77d9cec..9ff6158 100644 --- a/ssl/sslxprint.c +++ b/ssl/sslxprint.c @@ -73,6 +73,7 @@ int sslx_print_certificate(ssl,data,pf) #endif UCHAR *d; int _status; + struct json_object *cert_obj; #ifdef OPENSSL P_(P_ASN){ @@ -86,7 +87,6 @@ int sslx_print_certificate(ssl,data,pf) struct json_object *jobj; jobj = ssl->cur_json_st; - struct json_object *cert_obj; cert_obj = json_object_new_object(); d=data->data; @@ -183,7 +183,8 @@ int sslx_print_certificate(ssl,data,pf) abort: #ifdef OPENSSL if(x) X509_free(x); -#endif +#endif + if(cert_obj) json_object_put(cert_obj); return(_status); } From 9d3974b85f3cdb081c742ef2b70181ede4df0bfe Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 18:42:07 +0100 Subject: [PATCH 21/22] Avoid leak in TCP segment reassembly code --- base/tcppack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/tcppack.c b/base/tcppack.c index 1680e79..11c0edd 100644 --- a/base/tcppack.c +++ b/base/tcppack.c @@ -365,7 +365,8 @@ static int process_data_segment(conn,handler,p,stream,direction) else conn->state=TCP_STATE_CLOSED; } - + + free_tcp_segment_queue(stream->oo_queue); stream->oo_queue=seg->next; seg->next=0; stream->seq=seg->s_seq + seg->len; From 5e7d876af25fd85fd90696425602623998c867ec Mon Sep 17 00:00:00 2001 From: William Robinet Date: Wed, 27 Jan 2021 20:18:22 +0100 Subject: [PATCH 22/22] Fix bug introduced in 64effa3bb93c3a219fb0afd868c5bc2609093ced --- ssl/sslxprint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssl/sslxprint.c b/ssl/sslxprint.c index 9ff6158..4127eb4 100644 --- a/ssl/sslxprint.c +++ b/ssl/sslxprint.c @@ -184,7 +184,7 @@ int sslx_print_certificate(ssl,data,pf) #ifdef OPENSSL if(x) X509_free(x); #endif - if(cert_obj) json_object_put(cert_obj); + if(_status && cert_obj) json_object_put(cert_obj); return(_status); }