Skip to content

Commit 2607ba4

Browse files
douzzerdouzzer
authored
Merge pull request #83 from JeremiahM37/fenrir-fixes-3
Harden routing, dispatch, leaks, and parser correctness
2 parents 6c020a3 + 7e7a348 commit 2607ba4

6 files changed

Lines changed: 54 additions & 24 deletions

File tree

‎src/actions.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,11 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_action_get_reference(WOLFSENTRY_C
240240
label_len = (int)strlen(label);
241241
if (label_len > WOLFSENTRY_MAX_LABEL_BYTES)
242242
WOLFSENTRY_ERROR_RETURN(STRING_ARG_TOO_LONG);
243+
WOLFSENTRY_SHARED_OR_RETURN();
243244
if ((action_template = (struct wolfsentry_action *)WOLFSENTRY_MALLOC(sizeof *action_template + (size_t)label_len)) == NULL)
244-
WOLFSENTRY_ERROR_RETURN(SYS_RESOURCE_FAILED);
245+
WOLFSENTRY_ERROR_UNLOCK_AND_RETURN(SYS_RESOURCE_FAILED);
245246
action_template->label_len = (byte)label_len;
246247
memcpy(action_template->label, label, (size_t)label_len);
247-
WOLFSENTRY_SHARED_OR_RETURN();
248248
ret = wolfsentry_action_get_reference_1(WOLFSENTRY_CONTEXT_ARGS_OUT, action_template, action);
249249
WOLFSENTRY_FREE(action_template);
250250
WOLFSENTRY_ERROR_UNLOCK_AND_RERETURN(ret);
@@ -474,7 +474,10 @@ WOLFSENTRY_LOCAL wolfsentry_errcode_t wolfsentry_action_list_clone(
474474

475475
new_ale->action = new_action;
476476
WOLFSENTRY_REFCOUNT_INCREMENT(new_action->header.refcount, ret);
477-
WOLFSENTRY_UNLOCK_AND_RERETURN_IF_ERROR(ret);
477+
if (ret < 0) {
478+
WOLFSENTRY_FREE_1(dest_context->hpi.allocator, new_ale);
479+
goto out;
480+
}
478481
wolfsentry_list_ent_append(&dest_action_list->header, &new_ale->header);
479482
}
480483
ret = WOLFSENTRY_ERROR_ENCODE(OK);

‎src/json/centijson_sax.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,12 @@ json_buf_append(JSON_PARSER* parser, const unsigned char* data, size_t size)
311311
{
312312
if(parser->buf_used + size > parser->buf_alloced) {
313313
unsigned char* new_buf;
314-
size_t new_alloced = (parser->buf_used + size) * 2;
314+
size_t new_alloced;
315+
if(parser->buf_used > SIZE_MAX / 2 || size > SIZE_MAX / 2 - parser->buf_used) {
316+
json_raise(parser, JSON_ERR_OUTOFMEMORY);
317+
WOLFSENTRY_RETURN_VALUE(-1);
318+
}
319+
new_alloced = (parser->buf_used + size) * 2;
315320

316321
new_buf = (unsigned char *)realloc(parser->buf, new_alloced);
317322
if(new_buf == NULL) {

‎src/json/load_config.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,16 +368,22 @@ static wolfsentry_errcode_t convert_eventconfig_flag(JSON_TYPE type, wolfsentry_
368368

369369
static wolfsentry_errcode_t convert_wolfsentry_duration(struct wolfsentry_context *wolfsentry, JSON_TYPE type, const unsigned char *data, size_t data_size, wolfsentry_time_t *out) {
370370
wolfsentry_errcode_t ret;
371+
char buf[24];
371372
char *endptr;
372373
long conv;
373374

374375
if ((type != JSON_STRING) && (type != JSON_NUMBER))
375376
WOLFSENTRY_ERROR_RETURN(CONFIG_INVALID_VALUE);
376377

378+
if (data_size >= sizeof buf)
379+
WOLFSENTRY_ERROR_RETURN(NUMERIC_ARG_TOO_BIG);
380+
memcpy(buf, data, data_size);
381+
buf[data_size] = 0;
382+
377383
#ifndef WOLFSENTRY_NO_ERRNO_H
378384
errno = 0;
379385
#endif
380-
conv = strtol((const char *)data, &endptr, 0);
386+
conv = strtol(buf, &endptr, 0);
381387
#ifndef WOLFSENTRY_NO_ERRNO_H
382388
if (errno != 0)
383389
WOLFSENTRY_ERROR_RETURN(CONFIG_INVALID_VALUE);
@@ -405,7 +411,7 @@ static wolfsentry_errcode_t convert_wolfsentry_duration(struct wolfsentry_contex
405411
default:
406412
break;
407413
}
408-
if ((size_t)(endptr - (char *)data) != data_size)
414+
if ((size_t)(endptr - buf) != data_size)
409415
WOLFSENTRY_ERROR_RETURN(CONFIG_INVALID_VALUE);
410416
if ((ret = wolfsentry_interval_from_seconds(wolfsentry, conv, 0 /* howlong_nsecs */, out)) < 0)
411417
WOLFSENTRY_ERROR_RERETURN(ret);
@@ -602,6 +608,10 @@ static inline int convert_hex_byte(const unsigned char **in, size_t *in_len, byt
602608
return d1;
603609
++(*in);
604610
--(*in_len);
611+
if (*in_len < 1) {
612+
*out = (byte)d1;
613+
return 0;
614+
}
605615
d2 = convert_hex_digit(**in);
606616
if (d2 < 0) {
607617
*out = (byte)d1;

‎src/kv.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,12 @@ WOLFSENTRY_LOCAL wolfsentry_errcode_t wolfsentry_kv_delete(
539539
struct wolfsentry_kv_pair_internal *kv_template;
540540
struct wolfsentry_kv_pair_internal *old = NULL;
541541
wolfsentry_errcode_t ret;
542-
if ((ret = wolfsentry_kv_new(WOLFSENTRY_CONTEXT_ARGS_OUT, key, key_len, 0 /* data_len */, &kv_template)) < 0)
543-
WOLFSENTRY_ERROR_RERETURN(ret);
544542

545543
WOLFSENTRY_MUTEX_OR_RETURN();
546544

545+
if ((ret = wolfsentry_kv_new(WOLFSENTRY_CONTEXT_ARGS_OUT, key, key_len, 0 /* data_len */, &kv_template)) < 0)
546+
WOLFSENTRY_ERROR_UNLOCK_AND_RERETURN(ret);
547+
547548
ret = wolfsentry_kv_get_1(WOLFSENTRY_CONTEXT_ARGS_OUT, kv_table, kv_template, &old);
548549
WOLFSENTRY_WARN_ON_FAILURE(wolfsentry_kv_drop_reference(WOLFSENTRY_CONTEXT_ARGS_OUT, kv_template, NULL));
549550
WOLFSENTRY_UNLOCK_AND_RERETURN_IF_ERROR(ret);

‎src/routes.c‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ static inline int addr_prefix_match_size(
134134
for (; ret < min_len; ++ret) {
135135
int byte_number = ret / 8;
136136
int bit_number = ret % 8;
137-
if ((a[byte_number] & (1U << bit_number)) != (b[byte_number] & (1U << bit_number)))
137+
if ((a[byte_number] & (0x80U >> bit_number)) != (b[byte_number] & (0x80U >> bit_number)))
138138
break;
139139
}
140140

@@ -1959,6 +1959,8 @@ static wolfsentry_errcode_t wolfsentry_route_lookup_1(
19591959
const size_t addr_buf_size = WOLFSENTRY_BITS_TO_BYTES(remote->addr_len) + WOLFSENTRY_BITS_TO_BYTES(local->addr_len);
19601960
struct wolfsentry_route *target;
19611961

1962+
if (addr_buf_size > (size_t)WOLFSENTRY_MAX_ADDR_BYTES * 2)
1963+
WOLFSENTRY_ERROR_RETURN(NUMERIC_ARG_TOO_BIG);
19621964
target = (struct wolfsentry_route *)alloca(sizeof(*target) + addr_buf_size);
19631965
#define LOOKUP_TARGET target
19641966
#endif
@@ -2780,7 +2782,10 @@ static wolfsentry_errcode_t wolfsentry_route_event_dispatch_1(
27802782
if ((rule_route->parent_event == NULL) && (route_table->default_event != NULL)) {
27812783
rule_route->parent_event = route_table->default_event;
27822784
WOLFSENTRY_REFCOUNT_INCREMENT(rule_route->parent_event->header.refcount, ret);
2783-
WOLFSENTRY_UNLOCK_AND_RERETURN_IF_ERROR(ret);
2785+
if (ret < 0) {
2786+
rule_route->parent_event = NULL;
2787+
goto just_free_resources;
2788+
}
27842789
}
27852790
}
27862791

@@ -4027,24 +4032,26 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_route_flag_assoc_by_name(const ch
40274032

40284033
static wolfsentry_errcode_t ws_itoa(int i, unsigned char **out, size_t *spc) {
40294034
int out_chars;
4030-
int digit_thresh;
4035+
unsigned int u;
4036+
unsigned int digit_thresh;
40314037
int neg;
40324038
if (i < 0) {
40334039
neg = 1;
4034-
i = -i;
4040+
u = -(unsigned int)i;
40354041
out_chars = 2;
40364042
} else {
40374043
neg = 0;
4044+
u = (unsigned int)i;
40384045
out_chars = 1;
40394046
}
40404047
for (digit_thresh = 10; ; digit_thresh *= 10) {
4041-
if (i >= digit_thresh)
4048+
if (u >= digit_thresh)
40424049
++out_chars;
40434050
else {
40444051
digit_thresh /= 10;
40454052
break;
40464053
}
4047-
if (digit_thresh == 1000000000)
4054+
if (digit_thresh == 1000000000U)
40484055
break;
40494056
}
40504057
if (*spc < (size_t)out_chars)
@@ -4053,8 +4060,8 @@ static wolfsentry_errcode_t ws_itoa(int i, unsigned char **out, size_t *spc) {
40534060
if (neg)
40544061
*(*out)++ = '-';
40554062
while (digit_thresh >= 1) {
4056-
int quotient = i / digit_thresh;
4057-
i %= digit_thresh;
4063+
unsigned int quotient = u / digit_thresh;
4064+
u %= digit_thresh;
40584065
digit_thresh /= 10;
40594066
*(*out)++ = (unsigned char)('0' + quotient);
40604067
}
@@ -4640,7 +4647,7 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_route_render(WOLFSENTRY_CONTEXT_A
46404647
}
46414648
}
46424649

4643-
wolfsentry_route_render_proto(r->sa_proto, r->flags, f);
4650+
WOLFSENTRY_RERETURN_IF_ERROR(wolfsentry_route_render_proto(r->sa_proto, r->flags, f));
46444651

46454652
if (r->parent_event != NULL) {
46464653
if (fprintf(f, ", ev = \"%.*s\"%s", (int)r->parent_event->label_len, r->parent_event->label, WOLFSENTRY_CHECK_BITS(r->flags, WOLFSENTRY_ROUTE_FLAG_PARENT_EVENT_WILDCARD) ? "[*]" : "") < 0)
@@ -4730,11 +4737,14 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_route_exports_render(WOLFSENTRY_C
47304737
ret = wolfsentry_addr_family_ntop(WOLFSENTRY_CONTEXT_ARGS_OUT, r->sa_family, &addr_family, &family_name);
47314738
if (WOLFSENTRY_ERROR_CODE_IS(ret, OK)) {
47324739
if (fprintf(f, ", AF = %s", family_name) < 0)
4733-
WOLFSENTRY_ERROR_RETURN(IO_FAILED);
4740+
ret = WOLFSENTRY_ERROR_ENCODE(IO_FAILED);
47344741
if (addr_family) {
4735-
if ((ret = wolfsentry_addr_family_drop_reference(WOLFSENTRY_CONTEXT_ARGS_OUT, addr_family, NULL /* action_results */ )) < 0)
4736-
WOLFSENTRY_ERROR_RERETURN(ret);
4742+
wolfsentry_errcode_t drop_ret = wolfsentry_addr_family_drop_reference(WOLFSENTRY_CONTEXT_ARGS_OUT, addr_family, NULL /* action_results */ );
4743+
if (drop_ret < 0)
4744+
WOLFSENTRY_ERROR_RERETURN(drop_ret);
47374745
}
4746+
if (ret < 0)
4747+
WOLFSENTRY_ERROR_RERETURN(ret);
47384748
} else
47394749
#endif
47404750
{
@@ -4743,7 +4753,7 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_route_exports_render(WOLFSENTRY_C
47434753
}
47444754
}
47454755

4746-
wolfsentry_route_render_proto(r->sa_proto, r->flags, f);
4756+
WOLFSENTRY_RERETURN_IF_ERROR(wolfsentry_route_render_proto(r->sa_proto, r->flags, f));
47474757

47484758
if (r->parent_event_label_len > 0) {
47494759
if (fprintf(f, ", ev = \"%.*s\"%s", (int)r->parent_event_label_len, r->parent_event_label, WOLFSENTRY_CHECK_BITS(r->flags, WOLFSENTRY_ROUTE_FLAG_PARENT_EVENT_WILDCARD) ? "[*]" : "") < 0)

‎src/wolfsentry_util.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,9 +666,9 @@ static void *wolfsentry_builtin_realloc(
666666
#ifdef WOLFSENTRY_MALLOC_DEBUG
667667
{
668668
void *ret = realloc(ptr, size);
669-
if ((ptr == null) && (ret != NULL))
669+
if ((ptr == NULL) && (ret != NULL))
670670
WOLFSENTRY_ATOMIC_INCREMENT(n_mallocs, 1);
671-
else if ((ptr != null) && (ret == NULL))
671+
else if ((ptr != NULL) && (ret == NULL))
672672
WOLFSENTRY_ATOMIC_DECREMENT(n_mallocs, 1);
673673
return ret;
674674
}
@@ -1775,7 +1775,6 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_lock_destroy(struct wolfsentry_rw
17751775
(lock->read_waiter_count != 0) ||
17761776
(lock->write_waiter_count != 0) ||
17771777
(lock->read2write_waiter_read_count != 0) ||
1778-
(lock->read2write_reservation_holder != WOLFSENTRY_THREAD_NO_ID) ||
17791778
(lock->promoted_at_count != 0))
17801779
{
17811780
WOLFSENTRY_WARN("attempt to destroy lock with corrupted state {%u,%d,%d,%d,%d,%d,%d}\n", (unsigned int)lock->state, lock->holder_count.read, lock->read_waiter_count, lock->write_waiter_count, lock->read2write_waiter_read_count, lock->read2write_reservation_holder != WOLFSENTRY_THREAD_NO_ID, lock->promoted_at_count);
@@ -3618,6 +3617,8 @@ WOLFSENTRY_API wolfsentry_errcode_t wolfsentry_eventconfig_check(
36183617
((config->route_private_data_alignment & (config->route_private_data_alignment - 1)) != 0) ||
36193618
(config->route_private_data_alignment > config->route_private_data_size)))
36203619
WOLFSENTRY_ERROR_RETURN(INVALID_ARG);
3620+
if (config->route_private_data_size > MAX_UINT_OF(((struct wolfsentry_route *)0)->data_addr_offset))
3621+
WOLFSENTRY_ERROR_RETURN(NUMERIC_ARG_TOO_BIG);
36213622
}
36223623

36233624
if (config->route_private_data_alignment > 0) {

0 commit comments

Comments
 (0)