From 599481b9494ff9fa298ab26fa5e1cd50d70a871d Mon Sep 17 00:00:00 2001
From: Wesley Shields <wxs@atarininja.org>
Date: Wed, 15 Jun 2022 03:57:48 -0400
Subject: [PATCH] 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.
---
 libyara/modules/dex/dex.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/libyara/modules/dex/dex.c b/libyara/modules/dex/dex.c
index 6c9fd499d5..daa38363a3 100644
--- a/libyara/modules/dex/dex.c
+++ b/libyara/modules/dex/dex.c
@@ -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(
@@ -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);
@@ -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
@@ -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
@@ -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);
@@ -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);
@@ -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);
@@ -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;
 
@@ -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",
@@ -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,