Skip to content

Commit 5bbafb3

Browse files
committed
Fix some memory errors triggered by allocation failures
Some low hanging fruits found using nallocfuzz. See: https://github.com/catenacyber/nallocfuzz See: google/oss-fuzz#9902 Most of these errors are quite trivial to fix; the only exception is the stuff in the uthash. If the insertion fails (because of an allocation failure), we need to avoid some memory leaks. But the only way to check if the `HASH_ADD_*` failed, is to perform a new lookup: a bit costly, but we don't use that code in any critical data-path.
1 parent 346bb26 commit 5bbafb3

File tree

7 files changed

+78
-25
lines changed

7 files changed

+78
-25
lines changed

example/reader_util.c

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,14 @@ u_int16_t max_pattern_len = 8;
116116

117117
/* *********************************************************** */
118118

119-
void ndpi_analyze_payload(struct ndpi_flow_info *flow,
120-
u_int8_t src_to_dst_direction,
121-
u_int8_t *payload,
122-
u_int16_t payload_len,
123-
u_int32_t packet_id) {
124-
struct payload_stats *ret;
125-
struct flow_id_stats *f;
126-
struct packet_id_stats *p;
119+
int ndpi_analyze_payload(struct ndpi_flow_info *flow,
120+
u_int8_t src_to_dst_direction,
121+
u_int8_t *payload,
122+
u_int16_t payload_len,
123+
u_int32_t packet_id) {
124+
struct payload_stats *ret, *ret_found;
125+
struct flow_id_stats *f, *f_found;
126+
struct packet_id_stats *p, *p_found;
127127

128128
#ifdef DEBUG_PAYLOAD
129129
u_int16_t i;
@@ -135,11 +135,11 @@ void ndpi_analyze_payload(struct ndpi_flow_info *flow,
135135
HASH_FIND(hh, pstats, payload, payload_len, ret);
136136
if(ret == NULL) {
137137
if((ret = (struct payload_stats*)ndpi_calloc(1, sizeof(struct payload_stats))) == NULL)
138-
return; /* OOM */
138+
return -1; /* OOM */
139139

140140
if((ret->pattern = (u_int8_t*)ndpi_malloc(payload_len)) == NULL) {
141141
ndpi_free(ret);
142-
return;
142+
return -1;
143143
}
144144

145145
memcpy(ret->pattern, payload, payload_len);
@@ -148,6 +148,13 @@ void ndpi_analyze_payload(struct ndpi_flow_info *flow,
148148

149149
HASH_ADD(hh, pstats, pattern[0], payload_len, ret);
150150

151+
HASH_FIND(hh, pstats, payload, payload_len, ret_found);
152+
if(ret_found == NULL) { /* The insertion failed (because of a memory allocation error) */
153+
ndpi_free(ret->pattern);
154+
ndpi_free(ret);
155+
return -1;
156+
}
157+
151158
#ifdef DEBUG_PAYLOAD
152159
printf("Added element [total: %u]\n", HASH_COUNT(pstats));
153160
#endif
@@ -159,20 +166,32 @@ void ndpi_analyze_payload(struct ndpi_flow_info *flow,
159166
HASH_FIND_INT(ret->flows, &flow->flow_id, f);
160167
if(f == NULL) {
161168
if((f = (struct flow_id_stats*)ndpi_calloc(1, sizeof(struct flow_id_stats))) == NULL)
162-
return; /* OOM */
169+
return -1; /* OOM */
163170

164171
f->flow_id = flow->flow_id;
165172
HASH_ADD_INT(ret->flows, flow_id, f);
173+
174+
HASH_FIND_INT(ret->flows, &flow->flow_id, f_found);
175+
if(f_found == NULL) { /* The insertion failed (because of a memory allocation error) */
176+
ndpi_free(f);
177+
return -1;
178+
}
166179
}
167180

168181
HASH_FIND_INT(ret->packets, &packet_id, p);
169182
if(p == NULL) {
170183
if((p = (struct packet_id_stats*)ndpi_calloc(1, sizeof(struct packet_id_stats))) == NULL)
171-
return; /* OOM */
184+
return -1; /* OOM */
172185
p->packet_id = packet_id;
173186

174187
HASH_ADD_INT(ret->packets, packet_id, p);
188+
189+
HASH_FIND_INT(ret->packets, &packet_id, p_found);
190+
if(p_found == NULL) { /* The insertion failed (because of a memory allocation error) */
191+
ndpi_free(p);
192+
}
175193
}
194+
return 0;
176195
}
177196

178197
/* *********************************************************** */
@@ -199,7 +218,12 @@ void ndpi_payload_analyzer(struct ndpi_flow_info *flow,
199218
for(i=0; i<scan_len; i++) {
200219
for(j=min_pattern_len; j <= max_pattern_len; j++) {
201220
if((i+j) < payload_len) {
202-
ndpi_analyze_payload(flow, src_to_dst_direction, &payload[i], j, packet_id);
221+
if(ndpi_analyze_payload(flow, src_to_dst_direction, &payload[i], j, packet_id) == -1) {
222+
#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
223+
/* Avoid too much logging while fuzzing */
224+
LOG(NDPI_LOG_ERROR, "Error ndpi_analyze_payload (allocation failure)\n");
225+
#endif
226+
}
203227
}
204228
}
205229
}
@@ -960,6 +984,12 @@ static struct ndpi_flow_info *get_ndpi_flow_info(struct ndpi_workflow * workflow
960984
if(enable_flow_stats) {
961985
newflow->entropy = ndpi_calloc(1, sizeof(struct ndpi_entropy));
962986
newflow->last_entropy = ndpi_calloc(1, sizeof(struct ndpi_entropy));
987+
if(!newflow->entropy || !newflow->last_entropy) {
988+
ndpi_tdelete(newflow, &workflow->ndpi_flows_root[idx], ndpi_workflow_node_cmp);
989+
ndpi_flow_info_free_data(newflow);
990+
ndpi_free(newflow);
991+
return(NULL);
992+
}
963993
newflow->entropy->src2dst_pkt_len[newflow->entropy->src2dst_pkt_count] = l4_data_len;
964994
newflow->entropy->src2dst_pkt_time[newflow->entropy->src2dst_pkt_count] = when;
965995
if(newflow->entropy->src2dst_pkt_count == 0) {

fuzz/fuzz_ds_ahocorasick.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
133133

134134
f = fopen("/dev/null", "w");
135135
ac_automata_dump(a, f);
136-
fclose(f);
136+
if (f)
137+
fclose(f);
137138

138139
ac_automata_get_stats(a, &stats);
139140

fuzz/fuzz_libinjection.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
1212
/* Libinjection: it wants null-terminated string */
1313

1414
query = malloc(size + 1);
15+
if (!query)
16+
return 0;
1517
memcpy(query, data, size);
1618
query[size] = '\0';
1719

fuzz/fuzz_process_packet.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <stdio.h>
66

77
struct ndpi_detection_module_struct *ndpi_info_mod = NULL;
8+
struct ndpi_flow_struct ndpi_flow;
89
static ndpi_serializer json_serializer = {};
910
static ndpi_serializer csv_serializer = {};
1011

@@ -18,24 +19,23 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
1819
ndpi_init_serializer(&csv_serializer, ndpi_serialization_format_csv);
1920
}
2021

21-
struct ndpi_flow_struct *ndpi_flow = ndpi_flow_malloc(SIZEOF_FLOW_STRUCT);
22-
memset(ndpi_flow, 0, SIZEOF_FLOW_STRUCT);
22+
memset(&ndpi_flow, 0, SIZEOF_FLOW_STRUCT);
2323
ndpi_protocol detected_protocol =
24-
ndpi_detection_process_packet(ndpi_info_mod, ndpi_flow, Data, Size, 0, NULL);
24+
ndpi_detection_process_packet(ndpi_info_mod, &ndpi_flow, Data, Size, 0, NULL);
2525
ndpi_protocol guessed_protocol =
26-
ndpi_detection_giveup(ndpi_info_mod, ndpi_flow, 1, &protocol_was_guessed);
26+
ndpi_detection_giveup(ndpi_info_mod, &ndpi_flow, 1, &protocol_was_guessed);
2727

2828
ndpi_reset_serializer(&json_serializer);
2929
ndpi_reset_serializer(&csv_serializer);
3030
if (protocol_was_guessed == 0)
3131
{
32-
ndpi_dpi2json(ndpi_info_mod, ndpi_flow, detected_protocol, &json_serializer);
33-
ndpi_dpi2json(ndpi_info_mod, ndpi_flow, detected_protocol, &csv_serializer);
32+
ndpi_dpi2json(ndpi_info_mod, &ndpi_flow, detected_protocol, &json_serializer);
33+
ndpi_dpi2json(ndpi_info_mod, &ndpi_flow, detected_protocol, &csv_serializer);
3434
} else {
35-
ndpi_dpi2json(ndpi_info_mod, ndpi_flow, guessed_protocol, &json_serializer);
36-
ndpi_dpi2json(ndpi_info_mod, ndpi_flow, guessed_protocol, &csv_serializer);
35+
ndpi_dpi2json(ndpi_info_mod, &ndpi_flow, guessed_protocol, &json_serializer);
36+
ndpi_dpi2json(ndpi_info_mod, &ndpi_flow, guessed_protocol, &csv_serializer);
3737
}
38-
ndpi_free_flow(ndpi_flow);
38+
ndpi_free_flow_data(&ndpi_flow);
3939

4040
return 0;
4141
}

src/lib/ndpi_main.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2509,6 +2509,9 @@ static int ndpi_add_host_ip_subprotocol(struct ndpi_detection_module_struct *ndp
25092509
u_int16_t port = 0; /* Format ip:8.248.73.247:443 */
25102510
char *double_column;
25112511

2512+
if(!ndpi_str->protocols_ptree)
2513+
return(-1);
2514+
25122515
if(ptr) {
25132516
ptr[0] = '\0';
25142517
ptr++;
@@ -3674,6 +3677,9 @@ int ndpi_add_ip_risk_mask(struct ndpi_detection_module_struct *ndpi_str,
36743677
char *ip, ndpi_risk mask) {
36753678
char *saveptr, *addr = strtok_r(ip, "/", &saveptr);
36763679

3680+
if(!ndpi_str->ip_risk_mask_ptree)
3681+
return(-3);
3682+
36773683
if(addr) {
36783684
char *cidr = strtok_r(NULL, "\n", &saveptr);
36793685
struct in_addr pin;
@@ -6483,6 +6489,9 @@ int ndpi_load_ip_category(struct ndpi_detection_module_struct *ndpi_str,
64836489
char *ptr;
64846490
char ipbuf[64];
64856491

6492+
if(!ndpi_str->custom_categories.ipAddresses_shadow)
6493+
return(-1);
6494+
64866495
strncpy(ipbuf, ip_address_and_mask, sizeof(ipbuf));
64876496
ipbuf[sizeof(ipbuf) - 1] = '\0';
64886497

@@ -6618,7 +6627,9 @@ int ndpi_fill_ip_protocol_category(struct ndpi_detection_module_struct *ndpi_str
66186627

66196628
ret->custom_category_userdata = NULL;
66206629

6621-
if(ndpi_str->custom_categories.categories_loaded) {
6630+
if(ndpi_str->custom_categories.categories_loaded &&
6631+
ndpi_str->custom_categories.ipAddresses) {
6632+
66226633
ndpi_prefix_t prefix;
66236634
ndpi_patricia_node_t *node;
66246635

src/lib/ndpi_utils.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,6 +2288,7 @@ int ndpi_hash_add_entry(ndpi_str_hash **h, char *key, u_int8_t key_len, void *va
22882288
{
22892289
struct ndpi_str_hash_private **h_priv = (struct ndpi_str_hash_private **)h;
22902290
struct ndpi_str_hash_private *new = ndpi_calloc(1, sizeof(*new));
2291+
struct ndpi_str_hash_private *found;
22912292
unsigned int hash_value;
22922293

22932294
if (new == NULL)
@@ -2299,6 +2300,14 @@ int ndpi_hash_add_entry(ndpi_str_hash **h, char *key, u_int8_t key_len, void *va
22992300
new->hash = hash_value;
23002301
new->value = value;
23012302
HASH_ADD_INT(*h_priv, hash, new);
2303+
2304+
HASH_FIND_INT(*h_priv, &hash_value, found);
2305+
if (found == NULL) /* The insertion failed (because of a memory allocation error) */
2306+
{
2307+
ndpi_free(new);
2308+
return 1;
2309+
}
2310+
23022311
return 0;
23032312
}
23042313

src/lib/third_party/include/uthash.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ do {
101101
#endif
102102

103103
#ifndef HASH_NONFATAL_OOM
104-
#define HASH_NONFATAL_OOM 0
104+
#define HASH_NONFATAL_OOM 1
105105
#endif
106106

107107
#if HASH_NONFATAL_OOM

0 commit comments

Comments
 (0)