Skip to content

Commit

Permalink
fuzz: improve coverage and remove dead code (#2135)
Browse files Browse the repository at this point in the history
We are not able to remove custom rules: remove the empty stubs (which
originate from the original OpenDPI code).

`ndpi_guess_protocol_id()` is only called on the first packet of the
flow, so the bitmask `flow->excluded_protocol_bitmask` is always empty,
since we didn't call any dissectors yet.

Move another hash function to the dedicated source file.
  • Loading branch information
IvanNardi authored Nov 7, 2023
1 parent 58a9e2d commit b539b0d
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 134 deletions.
3 changes: 3 additions & 0 deletions fuzz/fuzz_alg_crc32_md5.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
ndpi_murmur_hash((const char *)data, size);
ndpi_quick_hash(data, size);

if(size >= 16)
ndpi_quick_16_byte_hash(data);

str = ndpi_malloc(size + 1);
if(str) {
memcpy(str, data, size);
Expand Down
2 changes: 2 additions & 0 deletions fuzz/fuzz_ds_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
ndpi_hash_find_entry(h, value_added.data(), value_added.size(), &value);
}

if (fuzzed_data.ConsumeBool())
ndpi_hash_free(NULL, cleanup_func);
ndpi_hash_free(&h, cleanup_func);

return 0;
Expand Down
42 changes: 24 additions & 18 deletions fuzz/fuzz_gcrypt_cipher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,34 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
rc_e = mbedtls_cipher_setkey(ctx_e, key.data(), key.size() * 8, MBEDTLS_ENCRYPT);
rc_d = mbedtls_cipher_setkey(ctx_d, key.data(), key.size() * 8, MBEDTLS_DECRYPT);
if(rc_e == 0 && rc_d == 0) {
rc_e = mbedtls_cipher_set_iv(ctx_e, iv.data(), iv.size());
rc_d = mbedtls_cipher_set_iv(ctx_d, iv.data(), iv.size());
if(rc_e == 0 && rc_d == 0) {
mbedtls_cipher_reset(ctx_e);
mbedtls_cipher_reset(ctx_d);

rc_e = mbedtls_cipher_update(ctx_e, input.data(), input.size(), output, &output_size);
if(rc_e == 0) {
rc_e = mbedtls_cipher_finish(ctx_e, NULL, &output_size2);

if(fuzzed_data.ConsumeBool()) {
rc_e = mbedtls_cipher_crypt(ctx_e, iv.data(), iv.size(),
input.data(), input.size(), output, &output_size);
} else {
rc_e = mbedtls_cipher_set_iv(ctx_e, iv.data(), iv.size());
rc_d = mbedtls_cipher_set_iv(ctx_d, iv.data(), iv.size());
if(rc_e == 0 && rc_d == 0) {
mbedtls_cipher_reset(ctx_e);
mbedtls_cipher_reset(ctx_d);

rc_e = mbedtls_cipher_update(ctx_e, input.data(), input.size(), output, &output_size);
if(rc_e == 0) {
rc_e = mbedtls_cipher_finish(ctx_e, NULL, &output_size2);
if(rc_e == 0) {

rc_d = mbedtls_cipher_update(ctx_d, output, output_size, decrypted, &decrypted_size);
if(rc_d == 0) {
rc_d = mbedtls_cipher_finish(ctx_d, NULL, &output_size2);
/* TODO: decryption doesn't work with no-aesni data path!
Note that with MASAN, aesni is always disabled */
rc_d = mbedtls_cipher_update(ctx_d, output, output_size, decrypted, &decrypted_size);
if(rc_d == 0) {
rc_d = mbedtls_cipher_finish(ctx_d, NULL, &output_size2);
/* TODO: decryption doesn't work with no-aesni data path!
Note that with MASAN, aesni is always disabled */
#if 0
if(rc_d == 0) {
assert(input.size() == decrypted_size);
assert(memcmp(input.data(), decrypted, decrypted_size) == 0);
}
if(rc_d == 0) {
assert(input.size() == decrypted_size);
assert(memcmp(input.data(), decrypted, decrypted_size) == 0);
}
#endif
}
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions fuzz/fuzz_gcrypt_gcm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
int key_len, rc_e, rc_d;
mbedtls_cipher_id_t cipher;
unsigned char *tag;
int iv_len, tag_len, input_length, force_auth_tag_error;
int iv_len, tag_len, ad_len, input_length, force_auth_tag_error;

/* No real memory allocations involved */

if(fuzzed_data.remaining_bytes() < 1 + 4 + 512 / 8 +
1 + 64 + /* iv */
1 + /* tag_len */
1 + 17 + /* ad */
1 + 64 + /* input */
1 + /* force_auth_tag_error */
1 /* useless data: to be able to add the check with assert */)
Expand All @@ -50,6 +51,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
tag_len = fuzzed_data.ConsumeIntegralInRange(0, 17);
tag = (unsigned char *)malloc(tag_len);

ad_len = fuzzed_data.ConsumeIntegralInRange(0, 17);
std::vector<u_int8_t>ad = fuzzed_data.ConsumeBytes<uint8_t>(ad_len);

input_length = fuzzed_data.ConsumeIntegralInRange(16, 64);
std::vector<unsigned char>input = fuzzed_data.ConsumeBytes<u_int8_t>(input_length);
output = (unsigned char *)malloc(input_length);
Expand All @@ -71,7 +75,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
rc_e = mbedtls_gcm_crypt_and_tag(gcm_e_ctx, MBEDTLS_GCM_ENCRYPT,
input.size(),
iv.data(), iv.size(),
NULL, 0, /* TODO */
ad.data(), ad.size(),
input.data(),
output,
tag_len, tag);
Expand All @@ -85,7 +89,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
rc_d = mbedtls_gcm_auth_decrypt(gcm_d_ctx,
input.size(),
iv.data(), iv.size(),
NULL, 0, /* TODO */
ad.data(), ad.size(),
tag, tag_len,
output,
decrypted);
Expand Down
27 changes: 27 additions & 0 deletions src/lib/ndpi_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,31 @@ u_int32_t ndpi_hash_string_len(const char *str, u_int len) {
return(hash);
}

/* ******************************************************************** */

#define ROR64(x,r) (((x)>>(r))|((x)<<(64-(r))))

/*
'in_16_bytes_long` points to some 16 byte memory data to be hashed;
two independent 64-bit linear congruential generators are applied
results are mixed, scrambled and cast to 32-bit
*/
u_int32_t ndpi_quick_16_byte_hash(u_int8_t *in_16_bytes_long) {
u_int64_t a = *(u_int64_t*)(in_16_bytes_long + 0);
u_int64_t c = *(u_int64_t*)(in_16_bytes_long + 8);

// multipliers are taken from sprng.org, addends are prime
a = a * 0x2c6fe96ee78b6955 + 0x9af64480a3486659;
c = c * 0x369dea0f31a53f85 + 0xd0c6225445b76b5b;

// mix results
a += c;

// final scramble
a ^= ROR64(a, 13) ^ ROR64(a, 7);

// down-casting, also taking advantage of upper half
a ^= a >> 32;

return((u_int32_t)a);
}
82 changes: 8 additions & 74 deletions src/lib/ndpi_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,6 @@ static int addDefaultPort(struct ndpi_detection_module_struct *ndpi_str,
ndpi_port_range *range, ndpi_proto_defaults_t *def,
u_int8_t customUserProto, ndpi_default_ports_tree_node_t **root,
const char *_func, int _line);
static int removeDefaultPort(ndpi_port_range *range, ndpi_proto_defaults_t *def,
ndpi_default_ports_tree_node_t **root);

static void ndpi_reset_packet_line_info(struct ndpi_packet_struct *packet);
static void ndpi_int_change_protocol(struct ndpi_detection_module_struct *ndpi_str, struct ndpi_flow_struct *flow,
Expand Down Expand Up @@ -474,7 +472,6 @@ u_int8_t ndpi_is_subprotocol_informative(struct ndpi_detection_module_struct *nd
/* All dissectors that have calls to ndpi_match_host_subprotocol() */
case NDPI_PROTOCOL_DNS:
return(1);
break;

default:
return(0);
Expand Down Expand Up @@ -682,37 +679,6 @@ static int addDefaultPort(struct ndpi_detection_module_struct *ndpi_str,

/* ****************************************************** */

/*
NOTE
This function must be called with a semaphore set, this in order to avoid
changing the datastructures while using them
*/
static int removeDefaultPort(ndpi_port_range *range, ndpi_proto_defaults_t *def,
ndpi_default_ports_tree_node_t **root) {
ndpi_default_ports_tree_node_t node;
u_int16_t port;

for(port = range->port_low; port <= range->port_high; port++) {
ndpi_default_ports_tree_node_t *ret;

node.proto = def, node.default_port = port;

ret = (ndpi_default_ports_tree_node_t *)
ndpi_tdelete(&node, (void *) root,
ndpi_default_ports_tree_node_t_cmp); /* Add it to the tree */

if(ret != NULL) {
ndpi_free((ndpi_default_ports_tree_node_t *) ret);
return(0);
}
}

return(-1);
}

/* ****************************************************** */

/*
This is a function used to see if we need to
add a trailer $ in case the string is complete
Expand Down Expand Up @@ -833,19 +799,6 @@ static int ndpi_add_host_url_subprotocol(struct ndpi_detection_module_struct *nd

}

/* ****************************************************** */

/*
NOTE
This function must be called with a semaphore set, this in order to avoid
changing the datastructures while using them
*/
static int ndpi_remove_host_url_subprotocol(struct ndpi_detection_module_struct *ndpi_str, char *value, int protocol_id) {
NDPI_LOG_ERR(ndpi_str, "[NDPI] Missing implementation for proto %s/%d\n", value, protocol_id);
return(-1);
}

/* ******************************************************************** */

int ndpi_init_empty_app_protocol(ndpi_protocol_match const * const hostname_list,
Expand Down Expand Up @@ -4014,15 +3967,8 @@ u_int16_t ndpi_guess_protocol_id(struct ndpi_detection_module_struct *ndpi_str,
if(found != NULL) {
u_int16_t guessed_proto = found->proto->protoId;

/* We need to check if the guessed protocol isn't excluded by nDPI */
if(flow && (proto == IPPROTO_UDP) &&
NDPI_COMPARE_PROTOCOL_TO_BITMASK(flow->excluded_protocol_bitmask, guessed_proto) &&
is_udp_not_guessable_protocol(guessed_proto))
return(NDPI_PROTOCOL_UNKNOWN);
else {
*user_defined_proto = found->customUserProto;
return(guessed_proto);
}
*user_defined_proto = found->customUserProto;
return(guessed_proto);
}
} else {
/* No TCP/UDP */
Expand Down Expand Up @@ -4264,8 +4210,8 @@ int ndpi_add_trusted_issuer_dn(struct ndpi_detection_module_struct *ndpi_str, ch
}
/* ******************************************************************** */

int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
char *rule, u_int8_t do_add) {
static int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
char *rule) {
char *at, *proto, *elem;
ndpi_proto_defaults_t *def;
u_int subprotocol_id, i;
Expand Down Expand Up @@ -4330,11 +4276,6 @@ int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
def = NULL;

if(def == NULL) {
if(!do_add) {
/* We need to remove a rule */
NDPI_LOG_ERR(ndpi_str, "Unable to find protocol '%s': skipping rule '%s'\n", proto, rule);
return(-3);
} else {
ndpi_port_range ports_a[MAX_DEFAULT_PORTS], ports_b[MAX_DEFAULT_PORTS];
char *equal = strchr(proto, '=');
u_int16_t user_proto_id = ndpi_str->ndpi_num_supported_protocols;
Expand Down Expand Up @@ -4371,7 +4312,6 @@ int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
def = &ndpi_str->proto_defaults[ndpi_str->ndpi_num_supported_protocols];
subprotocol_id = ndpi_str->ndpi_num_supported_protocols;
ndpi_str->ndpi_num_supported_protocols++, ndpi_str->ndpi_num_custom_protocols++;
}
}

while((elem = strsep(&rule, ",")) != NULL) {
Expand Down Expand Up @@ -4445,11 +4385,8 @@ int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
else
range.port_low = range.port_high = atoi(&elem[4]);

if(do_add)
rc = addDefaultPort(ndpi_str, &range, def, 1 /* Custom user proto */,
rc = addDefaultPort(ndpi_str, &range, def, 1 /* Custom user proto */,
is_tcp ? &ndpi_str->tcpRoot : &ndpi_str->udpRoot, __FUNCTION__, __LINE__);
else
rc = removeDefaultPort(&range, def, is_tcp ? &ndpi_str->tcpRoot : &ndpi_str->udpRoot);

if(rc != 0) ret = rc;
} else if(is_ip) {
Expand All @@ -4458,11 +4395,8 @@ int ndpi_handle_rule(struct ndpi_detection_module_struct *ndpi_str,
if(rc != 0)
return(rc);
} else {
if(do_add)
ndpi_add_host_url_subprotocol(ndpi_str, value, subprotocol_id, NDPI_PROTOCOL_CATEGORY_UNSPECIFIED,
NDPI_PROTOCOL_ACCEPTABLE, 0);
else
ndpi_remove_host_url_subprotocol(ndpi_str, value, subprotocol_id);
ndpi_add_host_url_subprotocol(ndpi_str, value, subprotocol_id, NDPI_PROTOCOL_CATEGORY_UNSPECIFIED,
NDPI_PROTOCOL_ACCEPTABLE, 0);
}
}

Expand Down Expand Up @@ -4971,7 +4905,7 @@ int ndpi_load_protocols_file_fd(struct ndpi_detection_module_struct *ndpi_str, F

/* printf("Processing: \"%s\"\n", buffer); */

if(ndpi_handle_rule(ndpi_str, buffer, 1) != 0)
if(ndpi_handle_rule(ndpi_str, buffer) != 0)
NDPI_LOG_INFO(ndpi_str, "Discraded rule '%s'\n", buffer);
}

Expand Down
34 changes: 2 additions & 32 deletions src/lib/ndpi_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -2051,11 +2051,10 @@ const char* ndpi_risk2str(ndpi_risk_enum risk) {

case NDPI_TLS_ALPN_SNI_MISMATCH:
return("ALPN/SNI Mismatch");

case NDPI_MALWARE_HOST_CONTACTED:
return("Client contacted a malware host");
break;


default:
ndpi_snprintf(buf, sizeof(buf), "%d", (int)risk);
return(buf);
Expand Down Expand Up @@ -2201,35 +2200,6 @@ ndpi_http_method ndpi_http_str2method(const char* method, u_int16_t method_len)

/* ******************************************************************** */

#define ROR64(x,r) (((x)>>(r))|((x)<<(64-(r))))

/*
'in_16_bytes_long` points to some 16 byte memory data to be hashed;
two independent 64-bit linear congruential generators are applied
results are mixed, scrambled and cast to 32-bit
*/
u_int32_t ndpi_quick_16_byte_hash(u_int8_t *in_16_bytes_long) {
u_int64_t a = *(u_int64_t*)(in_16_bytes_long + 0);
u_int64_t c = *(u_int64_t*)(in_16_bytes_long + 8);

// multipliers are taken from sprng.org, addends are prime
a = a * 0x2c6fe96ee78b6955 + 0x9af64480a3486659;
c = c * 0x369dea0f31a53f85 + 0xd0c6225445b76b5b;

// mix results
a += c;

// final scramble
a ^= ROR64(a, 13) ^ ROR64(a, 7);

// down-casting, also taking advantage of upper half
a ^= a >> 32;

return((u_int32_t)a);
}

/* ******************************************************************** */

int ndpi_hash_init(ndpi_str_hash **h)
{
if (h == NULL)
Expand Down
Binary file modified tests/cfgs/default/pcap/stun.pcap
Binary file not shown.
Loading

0 comments on commit b539b0d

Please sign in to comment.