Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Dex fixes (#1728)
* Fix compiler warnings with dex debug mode.

* Fix crashes in dex module.

This commit fixes a few crashes in the dex module. There are actually three of
them:

The first is incorrect usage of "struct_fits_in_dex" caused by passing
"sizeof(code_item_t)" instead of just "code_item_t" as the third argument. In
the test case the pointer for code_item started in the bounds of the dex but
only the first 8 bytes were within bounds, and since
"sizeof(sizeof(code_item_t))" is less than 8 the check was passing. The fix here
is to pass just the struct type as the third argument.

The second crash was an off-by-one error when parsing a string. The check
ensured the string fits in the dex but was not including an extra byte which was
copied in the call to set_sized_string. Just like before, this was a case of a
string falling right on the end of a dex file.

The third crash was due to a missing "struct_fits_in_dex" check. We ended up
with a pointer to a map_item_t which was off the ends of the dex bounds.

With this commit all the test cases provided in the report are now passing. I
did a quick sweep of the module to make sure there were no other cases where we
were incorrectly using "struct_fits_in_dex" and didn't find any.

These were all documented at a private report via huntr.dev
(https://huntr.dev/bounties/007a7784-c211-4847-9cc3-aec38e7d5157/)

Found by @sudhackar.

Fixes #1726.
  • Loading branch information
wxsBSD committed Jun 15, 2022
1 parent e23ac0d commit 599481b
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions libyara/modules/dex/dex.c
Expand Up @@ -619,7 +619,7 @@ uint32_t load_encoded_field(
{
#ifdef DEBUG_DEX_MODULE
printf(
"[DEX]\tFIELD_NAME %s NAME_IDX 0x%x\n", field_name->c_string, name_idx);
"[DEX]\tFIELD_NAME %s NAME_IDX 0x%llx\n", field_name->c_string, name_idx);
#endif

set_sized_string(
Expand All @@ -643,7 +643,7 @@ uint32_t load_encoded_field(
{
#ifdef DEBUG_DEX_MODULE
printf(
"[DEX]\tCLASS_NAME %s CLASS_IDX 0x%x DESCRIPTOR_IDX 0x%x\n",
"[DEX]\tCLASS_NAME %s CLASS_IDX 0x%llx DESCRIPTOR_IDX 0x%llx\n",
class_name->c_string,
class_idx,
descriptor_idx);
Expand Down Expand Up @@ -748,7 +748,7 @@ uint32_t load_encoded_method(
return 0;

#ifdef DEBUG_DEX_MODULE
printf("[DEX]\tNAME_IDX 0x%x\n", name_idx);
printf("[DEX]\tNAME_IDX 0x%llx\n", name_idx);
#endif

#ifdef DEBUG_DEX_MODULE
Expand All @@ -768,7 +768,7 @@ uint32_t load_encoded_method(
{
#ifdef DEBUG_DEX_MODULE
printf(
"[DEX]\tMETHOD_NAME %s NAME_IDX 0x%x\n",
"[DEX]\tMETHOD_NAME %s NAME_IDX 0x%llx\n",
method_name->c_string,
name_idx);
#endif
Expand All @@ -794,7 +794,7 @@ uint32_t load_encoded_method(
{
#ifdef DEBUG_DEX_MODULE
printf(
"[DEX]\tCLASS_NAME %s CLASS_IDX 0x%x DESCRIPTOR_IDX:0x%x\n",
"[DEX]\tCLASS_NAME %s CLASS_IDX 0x%llx DESCRIPTOR_IDX:0x%llx\n",
class_name->c_string,
class_idx,
descriptor_idx);
Expand All @@ -821,7 +821,7 @@ uint32_t load_encoded_method(
{
#ifdef DEBUG_DEX_MODULE
printf(
"[DEX]\tPROTO_NAME %s CLASS_IDX 0x%x DESCRIPTOR_IDX:0x%x\n",
"[DEX]\tPROTO_NAME %s CLASS_IDX 0x%llx DESCRIPTOR_IDX:0x%llx\n",
proto_name->c_string,
class_idx,
descriptor_idx);
Expand All @@ -842,7 +842,7 @@ uint32_t load_encoded_method(
#endif

if (struct_fits_in_dex(
dex, dex->data + encoded_method.code_off, sizeof(code_item_t)))
dex, dex->data + encoded_method.code_off, code_item_t))
{
code_item_t* code_item =
(code_item_t*) (dex->data + encoded_method.code_off);
Expand Down Expand Up @@ -954,7 +954,7 @@ void dex_parse(DEX* dex, uint64_t base_address)

if (!fits_in_dex(
dex,
dex->data + yr_le32toh(string_id_item->string_data_offset),
dex->data + yr_le32toh(string_id_item->string_data_offset) + 1,
value))
continue;

Expand All @@ -967,8 +967,8 @@ void dex_parse(DEX* dex, uint64_t base_address)
set_integer(value, dex->object, "string_ids[%i].size", i);

set_sized_string(
(const char*) ((
dex->data + yr_le32toh(string_id_item->string_data_offset) + 1)),
(const char*) (
dex->data + yr_le32toh(string_id_item->string_data_offset) + 1),
value,
dex->object,
"string_ids[%i].value",
Expand Down Expand Up @@ -1124,6 +1124,9 @@ void dex_parse(DEX* dex, uint64_t base_address)
map_item_t* map_item =
(map_item_t*) (dex->data + yr_le32toh(dex_header->map_offset) + sizeof(uint32_t) + i * sizeof(map_item_t));

if (!struct_fits_in_dex(dex, map_item, map_item_t))
return;

set_integer(
yr_le16toh(map_item->type),
dex->object,
Expand Down

0 comments on commit 599481b

Please sign in to comment.