Skip to content

Commit

Permalink
fix: check nullity of first block in math module
Browse files Browse the repository at this point in the history
This module retrieves the first block of memory to do some validation on
parameters. However, contrary to for example the hash module, the block
value is not checked. This can lead, if for example scanning a process
and having the process die during the scan, to a SIGSEGV/Null access
violation, as the first block call will return NULL.

This bug manifests in surprising ways. Since yara runs by default
evaluation of condition in a try catch block, this bug is actually
caught by the try catch block, and a surprising "ERROR_COULD_NOT_MAP_FILE"
error is then returned.

In addition, since the try catch bypasses the whole stack, this leads to
a memory leak that can add up as more instances of this bug is triggered.
  • Loading branch information
vthib committed Nov 29, 2023
1 parent 6bd2bc0 commit 4ec36ed
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
11 changes: 7 additions & 4 deletions docs/writingmodules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -657,11 +657,14 @@ checks in your code nevertheless). In those cases you can use the
const uint8_t* block_data;
block = first_memory_block(context);
block_data = block->fetch_data(block)
if (block_data != NULL)
if (block != NULL)
{
..do something with the memory block
block_data = block->fetch_data(block)
if (block_data != NULL)
{
..do something with the memory block
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions libyara/modules/magic/magic.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ define_function(magic_mime_type)
if (cache->cached_mime_type == NULL)
{
block = first_memory_block(context);

if (block == NULL)
return_string(YR_UNDEFINED);

block_data = block->fetch_data(block);

if (block_data != NULL)
Expand Down Expand Up @@ -134,6 +138,10 @@ define_function(magic_type)
if (cache->cached_type == NULL)
{
block = first_memory_block(context);

if (block == NULL)
return_string(YR_UNDEFINED);

block_data = block->fetch_data(block);

if (block_data != NULL)
Expand Down
11 changes: 8 additions & 3 deletions libyara/modules/math/math.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ uint32_t* get_distribution(int64_t offset, int64_t length, YR_SCAN_CONTEXT* cont
YR_MEMORY_BLOCK* block = first_memory_block(context);
YR_MEMORY_BLOCK_ITERATOR* iterator = context->iterator;

if (offset < 0 || length < 0 || offset < block->base)
if (block == NULL || offset < 0 || length < 0 || offset < block->base)
{
yr_free(data);
return NULL;
Expand Down Expand Up @@ -132,9 +132,8 @@ uint32_t* get_distribution_global(YR_SCAN_CONTEXT* context) {
if (data == NULL)
return NULL;

YR_MEMORY_BLOCK* block = first_memory_block(context);
YR_MEMORY_BLOCK* block;
YR_MEMORY_BLOCK_ITERATOR* iterator = context->iterator;

foreach_memory_block(iterator, block)
{
if (expected_next_offset != block->base)
Expand Down Expand Up @@ -323,6 +322,9 @@ define_function(data_serial_correlation)
YR_MEMORY_BLOCK* block = first_memory_block(context);
YR_MEMORY_BLOCK_ITERATOR* iterator = context->iterator;

if (block == NULL)
return_float(YR_UNDEFINED);

double sccun = 0;
double sccfirst = 0;
double scclast = 0;
Expand Down Expand Up @@ -450,6 +452,9 @@ define_function(data_monte_carlo_pi)
YR_MEMORY_BLOCK* block = first_memory_block(context);
YR_MEMORY_BLOCK_ITERATOR* iterator = context->iterator;

if (block == NULL)
return_float(YR_UNDEFINED);

if (offset < 0 || length < 0 || offset < block->base)
return_float(YR_UNDEFINED);

Expand Down

0 comments on commit 4ec36ed

Please sign in to comment.