Skip to content

Commit

Permalink
Fix some warnings reported by CODESonar (#2227)
Browse files Browse the repository at this point in the history
Remove some unreached/duplicated code.

Add error checking for `atoi()` calls.

About `isdigit()` and similar functions. The warning reported is:
```
Negative Character Value help
isdigit() is invoked here with an argument of signed type char, but only
has defined behavior for int arguments that are either representable
as unsigned char or equal to the value of macro EOF(-1).
Casting the argument to unsigned char will avoid the undefined behavior.
In a number of libc implementations, isdigit() is implemented using lookup
tables (arrays): passing in a negative value can result in a read underrun.
```
Switching to our macros fix that.
Add a check to `check_symbols.sh` to avoid using the original functions
from libc.
  • Loading branch information
IvanNardi authored Jan 12, 2024
1 parent 0aea509 commit dd8be1f
Show file tree
Hide file tree
Showing 20 changed files with 72 additions and 85 deletions.
2 changes: 1 addition & 1 deletion example/ndpiReader.c
Original file line number Diff line number Diff line change
Expand Up @@ -1963,7 +1963,7 @@ static void printFlow(u_int32_t id, struct ndpi_flow_info *flow, u_int16_t threa
fprintf(out, "[Payload: ");

for(i=0; i<flow->flow_payload_len; i++)
fprintf(out, "%c", isspace(flow->flow_payload[i]) ? '.' : flow->flow_payload[i]);
fprintf(out, "%c", ndpi_isspace(flow->flow_payload[i]) ? '.' : flow->flow_payload[i]);

fprintf(out, "]");
}
Expand Down
6 changes: 3 additions & 3 deletions example/reader_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ int ndpi_analyze_payload(struct ndpi_flow_info *flow,
#ifdef DEBUG_PAYLOAD
u_int16_t i;
for(i=0; i<payload_len; i++)
printf("%c", isprint(payload[i]) ? payload[i] : '.');
printf("%c", ndpi_isprint(payload[i]) ? payload[i] : '.');
printf("\n");
#endif

Expand Down Expand Up @@ -247,15 +247,15 @@ static void print_payload_stat(struct payload_stats *p, FILE *out) {
fprintf(out, "\t[");

for(i=0; i<p->pattern_len; i++) {
fprintf(out, "%c", isprint(p->pattern[i]) ? p->pattern[i] : '.');
fprintf(out, "%c", ndpi_isprint(p->pattern[i]) ? p->pattern[i] : '.');
}

fprintf(out, "]");
for(; i<16; i++) fprintf(out, " ");
fprintf(out, "[");

for(i=0; i<p->pattern_len; i++) {
fprintf(out, "%s%02X", (i > 0) ? " " : "", isprint(p->pattern[i]) ? p->pattern[i] : '.');
fprintf(out, "%s%02X", (i > 0) ? " " : "", ndpi_isprint(p->pattern[i]) ? p->pattern[i] : '.');
}

fprintf(out, "]");
Expand Down
2 changes: 1 addition & 1 deletion src/lib/ndpi_analyze.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ float ndpi_data_average(struct ndpi_analyze_struct *s) {
if((!s) || (s->num_data_entries == 0))
return(0);

return((s->num_data_entries == 0) ? 0 : ((float)s->sum_total / (float)s->num_data_entries));
return((float)s->sum_total / (float)s->num_data_entries);
}

/* ********************************************************************************* */
Expand Down
6 changes: 3 additions & 3 deletions src/lib/ndpi_classify.c
Original file line number Diff line number Diff line change
Expand Up @@ -559,9 +559,9 @@ ndpi_classify (const unsigned short *pkt_len, const pkt_timeval *pkt_time,
void
ndpi_update_params (classifier_type_codes_t param_type, const char *param_file)
{
float param;
float param = 0.0;
FILE *fp;
int count = 0;
int count;

switch (param_type) {
case (SPLT_PARAM_TYPE):
Expand Down Expand Up @@ -711,7 +711,7 @@ ndpi_log_timestamp(char *log_ts, uint32_t log_ts_len)
{
pkt_timeval tv;
time_t nowtime;
struct tm nowtm_r;
struct tm nowtm_r = { 0 };
char tmbuf[NDPI_TIMESTAMP_LEN];

gettimeofday(&tv, NULL);
Expand Down
2 changes: 1 addition & 1 deletion src/lib/ndpi_domain_classify.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ bool ndpi_domain_classify_contains(ndpi_domain_classify *s,
if((!strcmp(dot, ".arpa")) || (!strcmp(dot, ".local"))) return(false);

/* This is a number or a numeric IP or similar */
if(isdigit(domain[len-1]) && isdigit(domain[0])) {
if(ndpi_isdigit(domain[len-1]) && isdigit(domain[0])) {
#ifdef DEBUG_CONTAINS
printf("[contains] %s INVALID\n", domain);
#endif
Expand Down
66 changes: 31 additions & 35 deletions src/lib/ndpi_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4539,7 +4539,7 @@ int ndpi_load_categories_file(struct ndpi_detection_module_struct *ndpi_str,
int load_categories_file_fd(struct ndpi_detection_module_struct *ndpi_str,
FILE *fd, void *user_data) {
char buffer[512], *line, *name, *category, *saveptr;
int len, num = 0;
int len, num = 0, cat_id;

if(!ndpi_str || !fd)
return(-1);
Expand All @@ -4562,12 +4562,17 @@ int load_categories_file_fd(struct ndpi_detection_module_struct *ndpi_str,
category = strtok_r(NULL, "\t", &saveptr);

if(category) {
int rc = ndpi_load_category(ndpi_str, name,
(ndpi_protocol_category_t) atoi(category),
user_data);
const char *errstrp;
cat_id = ndpi_strtonum(category, 1, NDPI_PROTOCOL_NUM_CATEGORIES - 1, &errstrp, 10);
if(errstrp == NULL) {

int rc = ndpi_load_category(ndpi_str, name,
(ndpi_protocol_category_t)cat_id,
user_data);

if(rc >= 0)
num++;
if(rc >= 0)
num++;
}
}
}
}
Expand Down Expand Up @@ -4634,7 +4639,7 @@ int ndpi_load_category_file(struct ndpi_detection_module_struct *ndpi_str,
line[i] = '\0';
break;
}
if (line[i] != '-' && line[i] != '.' && isalnum(line[i]) == 0
if (line[i] != '-' && line[i] != '.' && ndpi_isalnum(line[i]) == 0
/* non standard checks for the sake of compatibility */
&& line[i] != '_')
break;
Expand Down Expand Up @@ -4689,19 +4694,19 @@ int ndpi_load_categories_dir(struct ndpi_detection_module_struct *ndpi_str,

/* Check if the format is <proto it>_<string>.<extension> */
if((underscore = strchr(dp->d_name, '_')) != NULL) {
ndpi_protocol_category_t proto_id;
int cat_id;
const char *errstrp;

underscore[0] = '\0';
proto_id = (ndpi_protocol_category_t)atoi(dp->d_name);

if((proto_id > 0) && (proto_id < (u_int16_t)NDPI_LAST_IMPLEMENTED_PROTOCOL)) {
cat_id = ndpi_strtonum(dp->d_name, 1, NDPI_PROTOCOL_NUM_CATEGORIES - 1, &errstrp, 10);
if(errstrp == NULL) {
/* Valid file */
char path[512];

underscore[0] = '_';
snprintf(path, sizeof(path), "%s/%s", dir_path, dp->d_name);

if (ndpi_load_category_file(ndpi_str, path, proto_id) < 0) {
if (ndpi_load_category_file(ndpi_str, path, (ndpi_protocol_category_t)cat_id) < 0) {
NDPI_LOG_ERR(ndpi_str, "Failed to load '%s'\n", path);
failed_files++;
}else
Expand Down Expand Up @@ -5975,9 +5980,6 @@ int ndpi_handle_ipv6_extension_headers(u_int16_t l3len, const u_int8_t **l4ptr,

*nxt_hdr = (*l4ptr)[0];

if(*l4len < ehdr_len)
return(1);

*l4len -= ehdr_len;
(*l4ptr) += ehdr_len;
}
Expand Down Expand Up @@ -6033,8 +6035,7 @@ static u_int8_t ndpi_detection_get_l4_internal(struct ndpi_detection_module_stru
if(l3 == NULL || l3_len < sizeof(struct ndpi_iphdr))
return(1);

if((iph = (const struct ndpi_iphdr *) l3) == NULL)
return(1);
iph = (const struct ndpi_iphdr *) l3;

if(iph->version == IPVERSION && iph->ihl >= 5) {
NDPI_LOG_DBG2(ndpi_str, "ipv4 header\n");
Expand Down Expand Up @@ -6483,7 +6484,7 @@ void ndpi_connection_tracking(struct ndpi_detection_module_struct *ndpi_str,
for(i=0; (i<packet->payload_packet_len)
&& (flow->flow_payload_len < ndpi_str->max_payload_track_len); i++) {
flow->flow_payload[flow->flow_payload_len++] =
(isprint(packet->payload[i]) || isspace(packet->payload[i])) ? packet->payload[i] : '.';
(ndpi_isprint(packet->payload[i]) || ndpi_isspace(packet->payload[i])) ? packet->payload[i] : '.';
}
}
}
Expand Down Expand Up @@ -7019,7 +7020,7 @@ static void ndpi_reconcile_protocols(struct ndpi_detection_module_struct *ndpi_s
(MS Teams uses Skype as transport protocol for voice/video)
*/
case NDPI_PROTOCOL_MSTEAMS:
if(flow && (flow->l4_proto == IPPROTO_TCP)) {
if(flow->l4_proto == IPPROTO_TCP) {
// printf("====>> NDPI_PROTOCOL_MSTEAMS\n");

if(ndpi_str->msteams_cache)
Expand All @@ -7031,7 +7032,7 @@ static void ndpi_reconcile_protocols(struct ndpi_detection_module_struct *ndpi_s
break;

case NDPI_PROTOCOL_STUN:
if(flow && (flow->guessed_protocol_id_by_ip == NDPI_PROTOCOL_MICROSOFT_AZURE))
if(flow->guessed_protocol_id_by_ip == NDPI_PROTOCOL_MICROSOFT_AZURE)
ndpi_reconcile_msteams_udp(ndpi_str, flow, NDPI_PROTOCOL_STUN);
break;

Expand All @@ -7054,8 +7055,7 @@ static void ndpi_reconcile_protocols(struct ndpi_detection_module_struct *ndpi_s
When Teams is unable to communicate via UDP
it switches to TLS.TCP. Let's try to catch it
*/
if(flow
&& (flow->guessed_protocol_id_by_ip == NDPI_PROTOCOL_MICROSOFT_AZURE)
if((flow->guessed_protocol_id_by_ip == NDPI_PROTOCOL_MICROSOFT_AZURE)
&& (ret->master_protocol == NDPI_PROTOCOL_UNKNOWN)
&& ndpi_str->msteams_cache
) {
Expand Down Expand Up @@ -9081,7 +9081,7 @@ const char *ndpi_category_get_name(struct ndpi_detection_module_struct *ndpi_str
}

if((category >= NDPI_PROTOCOL_CATEGORY_CUSTOM_1) && (category <= NDPI_PROTOCOL_CATEGORY_CUSTOM_5)) {
switch(category) {
switch((int)category) {
case NDPI_PROTOCOL_CATEGORY_CUSTOM_1:
return(ndpi_str->custom_category_labels[0]);
case NDPI_PROTOCOL_CATEGORY_CUSTOM_2:
Expand All @@ -9092,13 +9092,9 @@ const char *ndpi_category_get_name(struct ndpi_detection_module_struct *ndpi_str
return(ndpi_str->custom_category_labels[3]);
case NDPI_PROTOCOL_CATEGORY_CUSTOM_5:
return(ndpi_str->custom_category_labels[4]);
case NDPI_PROTOCOL_NUM_CATEGORIES:
return("Code should not use this internal constant");
default:
return("Unspecified");
}
} else
return(categories[category]);
}
return(categories[category]);
}

/* ****************************************************** */
Expand Down Expand Up @@ -10178,7 +10174,7 @@ u_int8_t ends_with(struct ndpi_detection_module_struct *ndpi_struct,
/* ******************************************************************** */

static int ndpi_is_trigram_char(char c) {
if(isdigit(c) || (c == '.') || (c == '-'))
if(ndpi_isdigit(c) || (c == '.') || (c == '-'))
return(0);
else
return(1);
Expand Down Expand Up @@ -10242,7 +10238,7 @@ int ndpi_check_dga_name(struct ndpi_detection_module_struct *ndpi_str,
ndpi_match_string_subprotocol(ndpi_str, name, strlen(name), &ret_match) > 0)
return(0); /* Ignore DGA for known domain names */

if(isdigit((int)name[0])) {
if(ndpi_isdigit(name[0])) {
struct in_addr ip_addr;

ip_addr.s_addr = inet_addr(name);
Expand Down Expand Up @@ -10278,7 +10274,7 @@ int ndpi_check_dga_name(struct ndpi_detection_module_struct *ndpi_str,
if(tmp[j] == '.') {
num_dots++;
} else if(num_dots == 0) {
if(!isdigit((int)tmp[j]))
if(!ndpi_isdigit(tmp[j]))
first_element_is_numeric = 0;
}

Expand All @@ -10291,10 +10287,10 @@ int ndpi_check_dga_name(struct ndpi_detection_module_struct *ndpi_str,
} else
num_char_repetitions = 1, last_char = tmp[j];

if(isdigit((int)tmp[j])) {
if(ndpi_isdigit(tmp[j])) {
num_digits++;

if(((j+2)<(u_int)len) && isdigit((int)tmp[j+1]) && (tmp[j+2] == '.')) {
if(((j+2)<(u_int)len) && ndpi_isdigit(tmp[j+1]) && (tmp[j+2] == '.')) {
/* Check if there are too many digits */
if(num_digits < 4)
return(0); /* Double digits */
Expand Down Expand Up @@ -10393,7 +10389,7 @@ int ndpi_check_dga_name(struct ndpi_detection_module_struct *ndpi_str,
trigram_char_skip = 0;

for(i = 0; word[i+1] != '\0'; i++) {
if(isdigit((int)word[i]))
if(ndpi_isdigit(word[i]))
num_consecutive_digits++;
else {
if((num_word == 1) && (num_consecutive_digits > max_num_consecutive_digits_first_word))
Expand Down
2 changes: 1 addition & 1 deletion src/lib/ndpi_serializer.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ static int ndpi_is_number(const char *str, u_int32_t str_len) {
unsigned int i;

for(i = 0; i < str_len; i++)
if(!isdigit((int)str[i])) return(0);
if(!ndpi_isdigit(str[i])) return(0);

return(1);
}
Expand Down
8 changes: 4 additions & 4 deletions src/lib/ndpi_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ static inline int ndpi_is_other_char(char c) {
/* ******************************************************************** */

static int _ndpi_is_valid_char(char c) {
if(ispunct(c) && (!ndpi_is_other_char(c)))
if(ndpi_ispunct(c) && (!ndpi_is_other_char(c)))
return(0);
else
return(ndpi_isdigit(c)
Expand All @@ -722,7 +722,7 @@ static int ndpi_find_non_eng_bigrams(struct ndpi_detection_module_struct *ndpi_s
char *str) {
char s[3];

if((isdigit((int)str[0]) && isdigit((int)str[1]))
if((ndpi_isdigit(str[0]) && ndpi_isdigit(str[1]))
|| ndpi_is_other_char(str[0])
|| ndpi_is_other_char(str[1])
)
Expand All @@ -741,7 +741,7 @@ int ndpi_has_human_readeable_string(struct ndpi_detection_module_struct *ndpi_st
char *buffer, u_int buffer_size,
u_int8_t min_string_match_len,
char *outbuf, u_int outbuf_len) {
u_int ret = 0, i = 0, do_cr = 0, len = 0, o_idx = 0, being_o_idx = 0;
u_int ret = 0, i, do_cr = 0, len = 0, o_idx = 0, being_o_idx = 0;

if(buffer_size <= 0)
return(0);
Expand All @@ -768,7 +768,7 @@ int ndpi_has_human_readeable_string(struct ndpi_detection_module_struct *ndpi_st
len += 1;
}

// printf("->> %c%c\n", isprint(buffer[i]) ? buffer[i] : '.', isprint(buffer[i+1]) ? buffer[i+1] : '.');
// printf("->> %c%c\n", ndpi_isprint(buffer[i]) ? buffer[i] : '.', ndpi_isprint(buffer[i+1]) ? buffer[i+1] : '.');
if(do_cr) {
if(len > min_string_match_len)
ret = 1;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/protocols/checkmk.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static void ndpi_search_checkmk(struct ndpi_detection_module_struct *ndpi_struct
* this will detect the OpenSession command of the Data Stream Interface (DSI) protocol
* which is exclusively used by the Apple Filing Protocol (AFP) on TCP/IP networks
*/
if (packet->payload_packet_len >= 15 && packet->payload_packet_len < 100
if (packet->payload_packet_len < 100
&& memcmp(packet->payload, "<<<check_mk>>>", 14) == 0) {

NDPI_LOG_DBG(ndpi_struct, "Check_MK: Flow detected.\n");
Expand Down
2 changes: 1 addition & 1 deletion src/lib/protocols/dns.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ static u_int8_t ndpi_grab_dns_name(struct ndpi_packet_struct *packet,

hostname_is_valid = 0;

if (isprint(c) == 0) {
if (ndpi_isprint(c) == 0) {
_hostname[j++] = '?';
} else {
_hostname[j++] = '_';
Expand Down
12 changes: 6 additions & 6 deletions src/lib/protocols/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ static int ndpi_search_http_tcp_again(struct ndpi_detection_module_struct *ndpi_
/* *********************************************** */

static int ndpi_http_is_print(char c) {
if(isprint(c) || (c == '\t') || (c == '\r') || (c == '\n'))
if(ndpi_isprint(c) || (c == '\t') || (c == '\r') || (c == '\n'))
return(1);
else
return(0);
Expand Down Expand Up @@ -568,11 +568,11 @@ static void ndpi_check_user_agent(struct ndpi_detection_module_struct *ndpi_stru
* We assume at least one non alpha char.
* e.g. ' ', '-' or ';' ...
*/
if (isalpha(ua[i]) == 0)
if (ndpi_isalpha(ua[i]) == 0)
{
break;
}
if (isupper(ua[i]) != 0)
if (isupper((unsigned char)ua[i]) != 0)
{
upper_case_count++;
}
Expand Down Expand Up @@ -771,7 +771,7 @@ static void ndpi_check_http_server(struct ndpi_detection_module_struct *ndpi_str
char buf[16] = { '\0' };

for(i=off, j=0; (i<server_len) && (j<sizeof(buf)-1)
&& (isdigit(server[i]) || (server[i] == '.')); i++)
&& (ndpi_isdigit(server[i]) || (server[i] == '.')); i++)
buf[j++] = server[i];

if(sscanf(buf, "%d.%d.%d", &a, &b, &c) == 3) {
Expand All @@ -790,7 +790,7 @@ static void ndpi_check_http_server(struct ndpi_detection_module_struct *ndpi_str

/* Check server content */
for(i=0; i<server_len; i++) {
if(!isprint(server[i])) {
if(!ndpi_isprint(server[i])) {
ndpi_set_risk(ndpi_struct, flow, NDPI_HTTP_SUSPICIOUS_HEADER, "Suspicious Agent");
break;
}
Expand All @@ -816,7 +816,7 @@ static void check_content_type_and_change_protocol(struct ndpi_detection_module_
&& (packet->host_line.len > 0)) {
int len = packet->http_url_name.len + packet->host_line.len + 1;

if(isdigit(packet->host_line.ptr[0])
if(ndpi_isdigit(packet->host_line.ptr[0])
&& (packet->host_line.len < 21))
ndpi_check_numeric_ip(ndpi_struct, flow, (char*)packet->host_line.ptr, packet->host_line.len);

Expand Down
2 changes: 1 addition & 1 deletion src/lib/protocols/kerberos.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ static void ndpi_search_kerberos(struct ndpi_detection_module_struct *ndpi_struc

name_offset += 1;
if(name_offset < packet->payload_packet_len - 1 &&
isprint(packet->payload[name_offset+1]) == 0) /* Isn't printable ? */
ndpi_isprint(packet->payload[name_offset+1]) == 0) /* Isn't printable ? */
{
name_offset++;
}
Expand Down
Loading

0 comments on commit dd8be1f

Please sign in to comment.