Skip to content

Commit f90cd22

Browse files
committed
Use indexes within a table of functions instead of putting function pointers directly in the YARA's VM stack.
This helps increasing the security of the VM machine.
1 parent 81adf51 commit f90cd22

File tree

2 files changed

+41
-12
lines changed

2 files changed

+41
-12
lines changed

libyara/exec.c

+39-10
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,24 @@ static int iter_string_set_next(YR_ITERATOR* self, YR_VALUE_STACK* stack)
364364
return ERROR_SUCCESS;
365365
}
366366

367+
// Global table that contains the "next" function for different types of
368+
// iterators. The reason for using this table is to avoid storing pointers
369+
// in the YARA's VM stack. Instead of the pointers we store an index within
370+
// this table.
371+
static YR_ITERATOR_NEXT_FUNC iter_next_func_table[] = {
372+
iter_array_next,
373+
iter_dict_next,
374+
iter_int_range_next,
375+
iter_int_enum_next,
376+
iter_string_set_next,
377+
};
378+
379+
#define ITER_NEXT_ARRAY 0
380+
#define ITER_NEXT_DICT 1
381+
#define ITER_NEXT_INT_RANGE 2
382+
#define ITER_NEXT_INT_ENUM 3
383+
#define ITER_NEXT_STRING_SET 4
384+
367385
int yr_execute_code(YR_SCAN_CONTEXT* context)
368386
{
369387
YR_DEBUG_FPRINTF(2, stderr, "+ %s() {\n", __FUNCTION__);
@@ -466,7 +484,7 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
466484
pop(r1);
467485
r2.it->array_it.array = r1.o;
468486
r2.it->array_it.index = 0;
469-
r2.it->next = iter_array_next;
487+
r2.it->next_func_idx = ITER_NEXT_ARRAY;
470488
push(r2);
471489
}
472490

@@ -487,7 +505,7 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
487505
pop(r1);
488506
r2.it->dict_it.dict = r1.o;
489507
r2.it->dict_it.index = 0;
490-
r2.it->next = iter_dict_next;
508+
r2.it->next_func_idx = ITER_NEXT_DICT;
491509
push(r2);
492510
}
493511

@@ -511,7 +529,7 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
511529
pop(r1);
512530
r3.it->int_range_it.next = r1.i;
513531
r3.it->int_range_it.last = r2.i;
514-
r3.it->next = iter_int_range_next;
532+
r3.it->next_func_idx = ITER_NEXT_INT_RANGE;
515533
push(r3);
516534
}
517535

@@ -537,7 +555,7 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
537555
{
538556
r3.it->int_enum_it.count = r1.i;
539557
r3.it->int_enum_it.next = 0;
540-
r3.it->next = iter_int_enum_next;
558+
r3.it->next_func_idx = ITER_NEXT_INT_ENUM;
541559

542560
for (int64_t i = r1.i; i > 0; i--)
543561
{
@@ -572,7 +590,7 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
572590
{
573591
r3.it->string_set_it.count = r1.i;
574592
r3.it->string_set_it.index = 0;
575-
r3.it->next = iter_string_set_next;
593+
r3.it->next_func_idx = ITER_NEXT_STRING_SET;
576594

577595
for (int64_t i = r1.i; i > 0; i--)
578596
{
@@ -594,11 +612,22 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
594612
// Loads the iterator in r1, but leaves the iterator in the stack.
595613
pop(r1);
596614
push(r1);
597-
// The iterator's next function is responsible for pushing the next
598-
// item in the stack, and a boolean indicating if there are more items
599-
// to retrieve. The boolean will be at the top of the stack after
600-
// calling "next".
601-
result = r1.it->next(r1.it, &stack);
615+
616+
if (r1.it->next_func_idx <
617+
sizeof(iter_next_func_table) / sizeof(YR_ITERATOR_NEXT_FUNC))
618+
{
619+
// The iterator's next function is responsible for pushing the next
620+
// item in the stack, and a boolean indicating if there are more items
621+
// to retrieve. The boolean will be at the top of the stack after
622+
// calling "next".
623+
result = iter_next_func_table[r1.it->next_func_idx](r1.it, &stack);
624+
}
625+
else
626+
{
627+
// next_func_idx is outside the valid range, this should not happend.
628+
result = ERROR_INTERNAL_FATAL_ERROR;
629+
}
630+
602631
stop = (result != ERROR_SUCCESS);
603632
break;
604633

libyara/include/yara/types.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,6 @@ struct RE_AST
397397
#pragma warning(disable : 4200)
398398
#endif
399399

400-
401400
// The RE structure is embedded in the YARA's VM instruction flow, which
402401
// means that its alignment is not guaranteed. For this reason the it must
403402
// be a "packed" structure, in order to prevent alignment issues in platforms
@@ -999,7 +998,8 @@ struct YR_STRING_SET_ITERATOR
999998

1000999
struct YR_ITERATOR
10011000
{
1002-
YR_ITERATOR_NEXT_FUNC next;
1001+
// Index of the next function within the iter_next_func_table global array.
1002+
int next_func_idx;
10031003

10041004
union
10051005
{

0 commit comments

Comments
 (0)