diff --git a/dp/ctrl.c b/dp/ctrl.c index faa37b03b..8e8d4441a 100644 --- a/dp/ctrl.c +++ b/dp/ctrl.c @@ -116,6 +116,8 @@ int dp_ctrl_send_json(json_t *root) } socklen_t addr_len = sizeof(struct sockaddr_un); + //data is nul terminated according to json_dumps + //so strlen(data) is safe here int sent = sendto(g_ctrl_fd, data, strlen(data), 0, (struct sockaddr *)&g_client_addr, addr_len); DEBUG_CTRL("%s\n", data); @@ -410,6 +412,7 @@ static int dp_ctrl_add_mac(json_t *msg) io_mac_t *mac, *ucmac, *bcmac; struct ether_addr oldmac; const char *mac_str, *ucmac_str, *bcmac_str, *oldmac_str, *pmac_str; + size_t ucl, bcl, oml, pml; const char *iface; int i, count=0; json_t *obj, *nw_obj; @@ -418,9 +421,17 @@ static int dp_ctrl_add_mac(json_t *msg) iface = json_string_value(json_object_get(msg, "iface")); mac_str = json_string_value(json_object_get(msg, "mac")); ucmac_str = json_string_value(json_object_get(msg, "ucmac")); + ucl = json_string_length(json_object_get(msg, "ucmac")); + bcmac_str = json_string_value(json_object_get(msg, "bcmac")); + bcl = json_string_length(json_object_get(msg, "bcmac")); + oldmac_str = json_string_value(json_object_get(msg, "oldmac")); + oml = json_string_length(json_object_get(msg, "oldmac")); + pmac_str = json_string_value(json_object_get(msg, "pmac")); + pml = json_string_length(json_object_get(msg, "pmac")); + obj = json_object_get(msg, "pips"); if (obj) { count = json_array_size(obj); @@ -460,23 +471,23 @@ static int dp_ctrl_add_mac(json_t *msg) ether_aton_r(mac_str, &mac->mac); mac->ep = ep; - if (strlen(ucmac_str) > 0) { + if (ucl > 0) { ether_aton_r(ucmac_str, &ucmac->mac); ucmac->unicast = 1; ucmac->ep = ep; } - if (strlen(bcmac_str) > 0) { + if (bcl > 0) { ether_aton_r(bcmac_str, &bcmac->mac); bcmac->broadcast = 1; bcmac->ep = ep; } - if (strlen(oldmac_str) > 0) { + if (oml > 0) { ether_aton_r(oldmac_str, &oldmac); } else { oldmac = mac->mac; } //for proxymesh ep, we need original ep's MAC to get policy handle - if (strlen(pmac_str) > 0) { + if (pml > 0) { ether_aton_r(pmac_str, &ep->pmac); } //proxymesh ep, we need original ep's IPs to do xff policy match diff --git a/dp/dpi/dpi_debug.c b/dp/dpi/dpi_debug.c index fc35a7328..2accc1ae7 100644 --- a/dp/dpi/dpi_debug.c +++ b/dp/dpi/dpi_debug.c @@ -37,6 +37,7 @@ void debug_log(bool print_ts, const char *fmt, ...) va_end(args); } +//debug purpose only not use in production void debug_dump_hex(const uint8_t *ptr, int len) { #define LINE_LEN (6 + 3 + 16 * 3 + 6 + 16) diff --git a/dp/dpi/dpi_entry.c b/dp/dpi/dpi_entry.c index 2fae64613..f5e254467 100755 --- a/dp/dpi/dpi_entry.c +++ b/dp/dpi/dpi_entry.c @@ -146,7 +146,7 @@ void dpi_ep_set_server_ver(dpi_packet_t *p, char *ver, int len) io_app_t *app = ep_app_map_locate(p->ep, s->server.port, s->ip_proto); if (unlikely(app == NULL)) return; - strncpy(app->version, ver, size); + strlcpy(app->version, ver, size); app->version[size] = '\0'; DEBUG_LOG(DBG_SESSION, p, "port=%u version=%s\n", s->server.port, app->version); diff --git a/dp/dpi/dpi_log.c b/dp/dpi/dpi_log.c index b9a3f8a59..1d53203ea 100644 --- a/dp/dpi/dpi_log.c +++ b/dp/dpi/dpi_log.c @@ -574,6 +574,7 @@ static void dpi_dlp_trigger_sig_dir(dpi_packet_t *p, dpi_match_t *m, bool flip, // Look for cache memset(&log, 0, sizeof(log)); log.ThreatID = htonl(tprop.id); + //sig->conf->name is null terminated log.DlpNameHash = sdbm_hash((uint8_t *)(sig->conf->name), strlen(sig->conf->name)*sizeof(char)); if (unlikely(p->ep_mac == NULL)) return; diff --git a/dp/dpi/dpi_policy.c b/dp/dpi/dpi_policy.c index 8809b33a1..ced84c475 100644 --- a/dp/dpi/dpi_policy.c +++ b/dp/dpi/dpi_policy.c @@ -1572,6 +1572,7 @@ static int fqdn_name_match(struct cds_lfht_node *ht_node, const void *key) static uint32_t fqdn_name_hash(const void *key) { + //key is null terminated const char *k = key; return sdbm_hash((uint8_t *)k, strlen(k)*sizeof(char)); } @@ -1813,6 +1814,7 @@ static bool match_fqdn_wildcard_name(char *vhost, char *fqdname) int i, j; strlcpy(dname, vhost, MAX_FQDN_LEN); + //dname is null terminated for (j = strlen(dname)-2; j > 0; j--) { if (dname[j] == '.') { break; @@ -1945,6 +1947,7 @@ int snooped_fqdn_ipv4_mapping(char *name, uint32_t *ip, int cnt) fqdn_name_entry_t *name_entry = NULL; dpi_fqdn_hdl_t *hdl = g_fqdn_hdl; + //name is null terminated if (strlen(name) > 0) { lower_string(name); } @@ -1960,6 +1963,7 @@ int snooped_fqdn_ipv4_mapping(char *name, uint32_t *ip, int cnt) int i, j; strlcpy(dname, name, MAX_FQDN_LEN); + //dname is null terminated for (j = strlen(dname)-2; j > 0; j--) { if (dname[j] == '.') { break; diff --git a/dp/dpi/parsers/dpi_dns.c b/dp/dpi/parsers/dpi_dns.c index a29571750..8d6d89cd2 100644 --- a/dp/dpi/parsers/dpi_dns.c +++ b/dp/dpi/parsers/dpi_dns.c @@ -277,7 +277,7 @@ static int dns_question(dpi_packet_t *p, uint8_t *ptr, int len, int shift, int c if (type == DNS_TYPE_A && questions != NULL) { if (jump > 1) { - strcpy(questions[(*qt_count)++].question, labels); + strlcpy(questions[(*qt_count)++].question, labels, MAX_LABEL_LEN); } }else if (type == DNS_TYPE_AXFR) { DEBUG_LOG(DBG_PARSER, p, "DNS Zone Transfer AXFR.\n"); @@ -323,7 +323,7 @@ static int dns_answer(dpi_packet_t *p, uint8_t *ptr, int len, int shift, int cou //ipv4 address if (type == DNS_TYPE_A && rd_len == 4 && answers != NULL) { if (jump > 1) { - strcpy(answers[*aw_count].question, labels); + strlcpy(answers[*aw_count].question, labels, MAX_LABEL_LEN); uint8_t *addr = ptr + shift; memcpy(&answers[*aw_count].ip4, addr, 4); answers[*aw_count].ip = true; @@ -334,8 +334,8 @@ static int dns_answer(dpi_packet_t *p, uint8_t *ptr, int len, int shift, int cou if (jump > 1) { int ret = get_dns_name(p, ptr, len, shift, cname); if (ret > 1) { - strcpy(answers[*aw_count].question, labels); - strcpy(answers[*aw_count].cname, cname); + strlcpy(answers[*aw_count].question, labels, MAX_LABEL_LEN); + strlcpy(answers[*aw_count].cname, cname, MAX_LABEL_LEN); answers[*aw_count].ip = false; (*aw_count)++; } diff --git a/dp/dpi/parsers/dpi_http.c b/dp/dpi/parsers/dpi_http.c index a701d7e25..c568e9df0 100644 --- a/dp/dpi/parsers/dpi_http.c +++ b/dp/dpi/parsers/dpi_http.c @@ -600,11 +600,13 @@ static int http_header_xforwarded_for_token(void *param, uint8_t *ptr, int len, } ip_str_len = l-ptr; + //preallocated memory ip_str = (char *) calloc(ip_str_len + 1, sizeof(char)); if (ip_str == NULL) { return CONSUME_TOKEN_SKIP_LINE; } strncpy(ip_str, (char *)ptr, ip_str_len); + //ip_str is null terminated ip_str[ip_str_len] = '\0'; s->xff_client_ip = inet_addr(ip_str); if (s->xff_client_ip == (uint32_t)(-1)) { @@ -616,6 +618,7 @@ static int http_header_xforwarded_for_token(void *param, uint8_t *ptr, int len, DEBUG_LOG(DBG_PARSER, p, "X-Forwarded-For: %s, ip=0x%08x, sess flags=0x%04x\n",ip_str, s->xff_client_ip, s->flags); + //memory freed free(ip_str); return CONSUME_TOKEN_SKIP_LINE; } @@ -642,11 +645,10 @@ static int http_header_host_token(void *param, uint8_t *ptr, int len, int token_ l ++; } host_str_len = l-ptr; + int size = min(host_str_len+1, sizeof(s->vhost)); + strlcpy((char *)s->vhost, (char *)ptr, size); - strncpy((char *)s->vhost, (char *)ptr, host_str_len); - s->vhost[host_str_len] = '\0'; - - s->vhlen = host_str_len; + s->vhlen = size-1; DEBUG_LOG(DBG_PARSER, p, "vhostname(%s) vhlen(%hu)\n", (char *)s->vhost, s->vhlen); return CONSUME_TOKEN_SKIP_LINE; diff --git a/dp/dpi/parsers/dpi_ssl.c b/dp/dpi/parsers/dpi_ssl.c index acf106ab4..ee7f3aa96 100644 --- a/dp/dpi/parsers/dpi_ssl.c +++ b/dp/dpi/parsers/dpi_ssl.c @@ -281,9 +281,9 @@ void ssl_get_sni_v3(dpi_packet_t *p, uint8_t *ptr, ssl_record_t *rec) uint16_t namelen = GET_BIG_INT16(tptr); tptr += 2; //DEBUG_LOG(DBG_PARSER, p, "ssl: snilen(%hu), sniname(%s)\n", namelen,(char *)tptr); - strncpy((char *)s->vhost, (char *)tptr, namelen); - s->vhost[namelen] = '\0'; - s->vhlen = namelen; + int size = min(namelen+1, sizeof(s->vhost)); + strlcpy((char *)s->vhost, (char *)tptr, size); + s->vhlen = size-1; tptr += namelen; break; } else { diff --git a/dp/dpi/parsers/dpi_tns.c b/dp/dpi/parsers/dpi_tns.c index 21f1991b5..96e7b17d8 100644 --- a/dp/dpi/parsers/dpi_tns.c +++ b/dp/dpi/parsers/dpi_tns.c @@ -101,8 +101,7 @@ static uint16_t tns_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uint16_t start = pos; SKIP_GROUP() - strncpy(prtcl, (char *)(tnsbodyptr + start), pos - start); - prtcl[pos - start] = '\0'; + strlcpy(prtcl, (char *)(tnsbodyptr + start), pos - start+1); DEBUG_LOG(DBG_PARSER, p, "tns: connect: protocol (%s)\n",prtcl); @@ -125,8 +124,7 @@ static uint16_t tns_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uint16_t } else { if (pos - start > 0) { char tmport[pos - start + 1]; - strncpy(tmport, (char *)(tnsbodyptr + start), pos - start); - tmport[pos - start] = '\0'; + strlcpy(tmport, (char *)(tnsbodyptr + start), pos - start + 1); DEBUG_LOG(DBG_PARSER, p, "tns: connect: connect port (%s) abnormal \n", tmport); dpi_fire_parser(p); *rc = TNS_NOTPASS; @@ -148,8 +146,7 @@ static uint16_t tns_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uint16_t SKIP_GROUP() - strncpy(tnshost, (char *)(tnsbodyptr + start), pos - start); - tnshost[pos - start] = '\0'; + strlcpy(tnshost, (char *)(tnsbodyptr + start), pos - start + 1); DEBUG_LOG(DBG_PARSER, p, "tns: connect: host (%s) \n", tnshost); } else { SKIP_GROUP() @@ -306,8 +303,7 @@ static uint16_t tns_redir_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uin start = pos; SKIP_GROUP() - strncpy(redirect_prtl, (char *)(tnsbodyptr + start), pos - start); - redirect_prtl[pos - start] = '\0'; + strlcpy(redirect_prtl, (char *)(tnsbodyptr + start), pos - start + 1); DEBUG_LOG(DBG_PARSER, p, "tns: redirect: connect protocol (%s)\n",redirect_prtl); @@ -331,8 +327,7 @@ static uint16_t tns_redir_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uin } else { if (pos - start > 0) { char tmport[pos - start + 1]; - strncpy(tmport, (char *)(tnsbodyptr + start), pos - start); - tmport[pos - start] = '\0'; + strlcpy(tmport, (char *)(tnsbodyptr + start), pos - start + 1); DEBUG_LOG(DBG_PARSER, p, "tns: redirect: connect port (%s) abnormal \n", tmport); } } @@ -350,8 +345,7 @@ static uint16_t tns_redir_connect_addr(dpi_packet_t *p, uint8_t *tnsbodyptr, uin SKIP_GROUP() - strncpy(host, (char *)(tnsbodyptr + start), pos - start); - host[pos - start] = '\0'; + strlcpy(host, (char *)(tnsbodyptr + start), pos - start + 1); DEBUG_LOG(DBG_PARSER, p, "tns: redirect: host (%s)\n", host); if ( pos - start > 6) { diff --git a/dp/dpi/sig/dpi_sig.c b/dp/dpi/sig/dpi_sig.c index c9bcfdc53..db1e0a9fd 100644 --- a/dp/dpi/sig/dpi_sig.c +++ b/dp/dpi/sig/dpi_sig.c @@ -5,7 +5,7 @@ uint32_t DlpRuleCount = 0; #define MAX_USER_SIG_COUNT (DPI_SIG_MAX_USER_SIG_ID - DPI_SIG_MIN_USER_SIG_ID + 1) -#define MAX_USER_SIG_LEN 1280//1024+256 +#define MAX_USER_SIG_LEN 2048 static dpi_sigopt_status_t dpi_dlp_parse_opts_routine (dpi_dlp_parser_t *parser, char **opts, int count, @@ -75,6 +75,7 @@ dpi_dlp_parse_opts_routine (dpi_dlp_parser_t *parser, char **opts, int count, DEBUG_ERROR(DBG_ERROR, "too many dlp rule (%d) created, max allowed is (%d) ", DlpRuleCount, MAX_USER_SIG_COUNT); return DPI_SIGOPT_TOO_MANY_DLP_RULE; } + //conf->text null terminated if (strlen(conf->text) >= MAX_USER_SIG_LEN) { DEBUG_ERROR(DBG_ERROR, "dlp rule len(%d) too long, max allowed is (%d) ", strlen(conf->text), MAX_USER_SIG_LEN); return DPI_SIGOPT_VALUE_TOO_LONG; @@ -195,6 +196,7 @@ dpi_dlp_parse_rule (dpi_dlp_parser_t *parser, char **opts, int count, dpi_sig_macro_sig_t *macro; dpi_sig_t *sig; int i; + //text is null terminated int text_len = strlen(text); for (i = 0; i < count; i ++) { diff --git a/dp/utils/helper.c b/dp/utils/helper.c index a3dab14ca..d3f828939 100755 --- a/dp/utils/helper.c +++ b/dp/utils/helper.c @@ -30,7 +30,7 @@ size_t strlcpy (char *dst, const char *src, size_t siz) if (n == 0) { if (siz != 0) *d = '\0'; /* NUL-terminate dst */ - while (*s++) + while (s&&*s++) /* avoid dereference a NUL pointer, get length of src*/ ; }