Skip to content

Commit

Permalink
NVSHAS-9541, Fix and check potential buffer overflow cases in c code
Browse files Browse the repository at this point in the history
  • Loading branch information
gfeng committed Oct 15, 2024
1 parent c3bac6b commit 6535c7d
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 30 deletions.
19 changes: 15 additions & 4 deletions dp/ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions dp/dpi/dpi_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion dp/dpi/dpi_entry.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions dp/dpi/dpi_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions dp/dpi/dpi_policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions dp/dpi/parsers/dpi_dns.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand All @@ -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)++;
}
Expand Down
10 changes: 6 additions & 4 deletions dp/dpi/parsers/dpi_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions dp/dpi/parsers/dpi_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 6 additions & 12 deletions dp/dpi/parsers/dpi_tns.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
Expand All @@ -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()
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
}
Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion dp/dpi/sig/dpi_sig.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ++) {
Expand Down
2 changes: 1 addition & 1 deletion dp/utils/helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
;
}

Expand Down

0 comments on commit 6535c7d

Please sign in to comment.