From 3c970a0a1b74a1991be303132221329f3eef0b91 Mon Sep 17 00:00:00 2001
From: Cyrille Bagard <nocbos@gmail.com>
Date: Sat, 24 Sep 2016 23:18:51 +0200
Subject: Prevented out of bounds access when moving the reading position
 forwards.

---
 ChangeLog                          | 26 +++++++++++++++++++++
 src/analysis/content-int.h         |  5 ++++
 src/analysis/content.c             | 25 ++++++++++++++++++++
 src/analysis/content.h             |  3 +++
 src/analysis/contents/file.c       | 47 ++++++++++++++++++++++++++++++++------
 src/analysis/contents/restricted.c | 47 ++++++++++++++++++++++++++++++++++++++
 src/analysis/disass/area.c         |  6 ++---
 src/arch/dalvik/operand.c          |  2 +-
 src/arch/dalvik/processor.c        |  8 +++++--
 src/arch/dalvik/pseudo/fill.c      |  5 +++-
 src/arch/dalvik/pseudo/switch.c    |  5 +++-
 src/format/dwarf/v2/form.c         |  6 ++++-
 12 files changed, 168 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6e8a942..2515f0e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,31 @@
 16-09-24  Cyrille Bagard <nocbos@gmail.com>
 
+	* src/analysis/content-int.h:
+	* src/analysis/content.c:
+	* src/analysis/content.h:
+	* src/analysis/contents/file.c:
+	* src/analysis/contents/restricted.c:
+	Prevent out of bounds access when moving the reading position forwards.
+
+	* src/analysis/disass/area.c:
+	Replace code by assertion.
+
+	* src/arch/dalvik/operand.c:
+	Update code.
+
+	* src/arch/dalvik/processor.c:
+	Restore the previous valid position in case of reading error for
+	decoding pseudo instructions.
+
+	* src/arch/dalvik/pseudo/fill.c:
+	* src/arch/dalvik/pseudo/switch.c:
+	Update code. Limit the quantity of displayed code.
+
+	* src/format/dwarf/v2/form.c:
+	Update code.
+
+16-09-24  Cyrille Bagard <nocbos@gmail.com>
+
 	* src/common/bits.c:
 	Replace the non-working GLib atomic function to deal with bitfields.
 
diff --git a/src/analysis/content-int.h b/src/analysis/content-int.h
index 24cea81..558e32c 100644
--- a/src/analysis/content-int.h
+++ b/src/analysis/content-int.h
@@ -41,6 +41,9 @@ typedef void (* compute_checksum_fc) (GBinContent *, GChecksum *);
 /* Détermine le nombre d'octets lisibles. */
 typedef phys_t (* compute_size_fc) (const GBinContent *);
 
+/* Avance la tête de lecture d'une certaine quantité de données. */
+typedef bool (* seek_fc) (const GBinContent *, vmpa2t *, phys_t);
+
 /* Donne accès à une portion des données représentées. */
 typedef const bin_t * (* get_raw_access_fc) (const GBinContent *, vmpa2t *, phys_t);
 
@@ -82,6 +85,8 @@ struct _GBinContentIface
 
     compute_size_fc compute_size;           /* Calcul de la taille totale  */
 
+    seek_fc seek;                           /* Avancée de tête de lecture  */
+
     get_raw_access_fc get_raw_access;       /* Accès brut à une position   */
 
     read_raw_fc read_raw;                   /* Lecture brute               */
diff --git a/src/analysis/content.c b/src/analysis/content.c
index 4b4645f..898bf43 100644
--- a/src/analysis/content.c
+++ b/src/analysis/content.c
@@ -216,6 +216,31 @@ phys_t g_binary_content_compute_size(const GBinContent *content)
 *                                                                             *
 *  Paramètres  : content = contenu binaire à venir lire.                      *
 *                addr    = position de la tête de lecture.                    *
+*                length  = quantité d'octets à provisionner.                  *
+*                                                                             *
+*  Description : Avance la tête de lecture d'une certaine quantité de données.*
+*                                                                             *
+*  Retour      : Bilan de l'opération : true en cas de succès, false sinon.   *
+*                                                                             *
+*  Remarques   : -                                                            *
+*                                                                             *
+******************************************************************************/
+
+bool g_binary_content_seek(const GBinContent *content, vmpa2t *addr, phys_t length)
+{
+    GBinContentIface *iface;                /* Interface utilisée          */
+
+    iface = G_BIN_CONTENT_GET_IFACE(content);
+
+    return iface->seek(content, addr, length);
+
+}
+
+
+/******************************************************************************
+*                                                                             *
+*  Paramètres  : content = contenu binaire à venir lire.                      *
+*                addr    = position de la tête de lecture.                    *
 *                length  = quantité d'octets à lire.                          *
 *                                                                             *
 *  Description : Donne accès à une portion des données représentées.          *
diff --git a/src/analysis/content.h b/src/analysis/content.h
index f1e5bbf..4ec4f61 100644
--- a/src/analysis/content.h
+++ b/src/analysis/content.h
@@ -69,6 +69,9 @@ const gchar *g_binary_content_get_checksum(GBinContent *);
 /* Détermine le nombre d'octets lisibles. */
 phys_t g_binary_content_compute_size(const GBinContent *);
 
+/* Avance la tête de lecture d'une certaine quantité de données. */
+bool g_binary_content_seek(const GBinContent *, vmpa2t *, phys_t);
+
 /* Donne accès à une portion des données représentées. */
 const bin_t *g_binary_content_get_raw_access(const GBinContent *, vmpa2t *, phys_t);
 
diff --git a/src/analysis/contents/file.c b/src/analysis/contents/file.c
index 95cfd5d..e7bebd0 100644
--- a/src/analysis/contents/file.c
+++ b/src/analysis/contents/file.c
@@ -85,6 +85,9 @@ static void g_file_content_compute_checksum(GFileContent *, GChecksum *);
 /* Détermine le nombre d'octets lisibles. */
 static phys_t g_file_content_compute_size(const GFileContent *);
 
+/* Avance la tête de lecture d'une certaine quantité de données. */
+static bool g_file_content_seek(const GFileContent *, vmpa2t *, phys_t);
+
 /* Donne accès à une portion des données représentées. */
 static const bin_t *g_file_content_get_raw_access(const GFileContent *, vmpa2t *, phys_t);
 
@@ -183,6 +186,8 @@ static void g_file_content_interface_init(GBinContentInterface *iface)
 
     iface->compute_size = (compute_size_fc)g_file_content_compute_size;
 
+    iface->seek = (seek_fc)g_file_content_seek;
+
     iface->get_raw_access = (get_raw_access_fc)g_file_content_get_raw_access;
 
     iface->read_raw = (read_raw_fc)g_file_content_read_raw;
@@ -479,31 +484,59 @@ static phys_t g_file_content_compute_size(const GFileContent *content)
 *                                                                             *
 *  Paramètres  : content = contenu binaire à venir lire.                      *
 *                addr    = position de la tête de lecture.                    *
-*                length  = quantité d'octets à lire.                          *
+*                length  = quantité d'octets à provisionner.                  *
 *                                                                             *
-*  Description : Donne accès à une portion des données représentées.          *
+*  Description : Avance la tête de lecture d'une certaine quantité de données.*
 *                                                                             *
-*  Retour      : Pointeur vers les données à lire ou NULL en cas d'échec.     *
+*  Retour      : Bilan de l'opération : true en cas de succès, false sinon.   *
 *                                                                             *
 *  Remarques   : -                                                            *
 *                                                                             *
 ******************************************************************************/
 
-static const bin_t *g_file_content_get_raw_access(const GFileContent *content, vmpa2t *addr, phys_t length)
+static bool g_file_content_seek(const GFileContent *content, vmpa2t *addr, phys_t length)
 {
     phys_t offset;                          /* Emplacement de départ       */
 
     offset = get_phy_addr(addr);
 
     if (offset == VMPA_NO_PHYSICAL)
-        return NULL;
+        return false;
 
     if ((offset + length) >= get_mrange_length(&content->range))
-        return NULL;
+        return false;
 
     advance_vmpa(addr, length);
 
-    return &content->data[offset];
+    return true;
+
+}
+
+
+/******************************************************************************
+*                                                                             *
+*  Paramètres  : content = contenu binaire à venir lire.                      *
+*                addr    = position de la tête de lecture.                    *
+*                length  = quantité d'octets à lire.                          *
+*                                                                             *
+*  Description : Donne accès à une portion des données représentées.          *
+*                                                                             *
+*  Retour      : Pointeur vers les données à lire ou NULL en cas d'échec.     *
+*                                                                             *
+*  Remarques   : -                                                            *
+*                                                                             *
+******************************************************************************/
+
+static const bin_t *g_file_content_get_raw_access(const GFileContent *content, vmpa2t *addr, phys_t length)
+{
+    phys_t offset;                          /* Emplacement de départ       */
+    bool allowed;                           /* Capacité d'avancer ?        */
+
+    offset = get_phy_addr(addr);
+
+    allowed = g_file_content_seek(content, addr, length);
+
+    return (allowed ? &content->data[offset] : NULL);
 
 }
 
diff --git a/src/analysis/contents/restricted.c b/src/analysis/contents/restricted.c
index e342242..1f996d1 100644
--- a/src/analysis/contents/restricted.c
+++ b/src/analysis/contents/restricted.c
@@ -69,6 +69,9 @@ static void g_restricted_content_finalize(GRestrictedContent *);
 /* Calcule une empreinte unique (SHA256) pour les données. */
 static void g_restricted_content_compute_checksum(GRestrictedContent *, GChecksum *);
 
+/* Avance la tête de lecture d'une certaine quantité de données. */
+static bool g_restricted_content_seek(const GRestrictedContent *, vmpa2t *, phys_t);
+
 /* Donne accès à une portion des données représentées. */
 static const bin_t *g_restricted_content_get_raw_access(const GRestrictedContent *, vmpa2t *, phys_t);
 
@@ -161,6 +164,8 @@ static void g_restricted_content_interface_init(GBinContentInterface *iface)
 {
     iface->compute_checksum = (compute_checksum_fc)g_restricted_content_compute_checksum;
 
+    iface->seek = (seek_fc)g_restricted_content_seek;
+
     iface->get_raw_access = (get_raw_access_fc)g_restricted_content_get_raw_access;
 
     iface->read_raw = (read_raw_fc)g_restricted_content_read_raw;
@@ -286,6 +291,48 @@ static void g_restricted_content_compute_checksum(GRestrictedContent *content, G
 *                                                                             *
 *  Paramètres  : content = contenu binaire à venir lire.                      *
 *                addr    = position de la tête de lecture.                    *
+*                length  = quantité d'octets à provisionner.                  *
+*                                                                             *
+*  Description : Avance la tête de lecture d'une certaine quantité de données.*
+*                                                                             *
+*  Retour      : Bilan de l'opération : true en cas de succès, false sinon.   *
+*                                                                             *
+*  Remarques   : -                                                            *
+*                                                                             *
+******************************************************************************/
+
+static bool g_restricted_content_seek(const GRestrictedContent *content, vmpa2t *addr, phys_t length)
+{
+    bool result;                            /* Bilan de lecture à renvoyer */
+    vmpa2t old;                             /* Copie de sauvegarde         */
+
+    if (!mrange_contains_addr(&content->range, addr))
+    {
+        result = false;
+        goto bad_range;
+    }
+
+    copy_vmpa(&old, addr);
+
+    result = g_binary_content_seek(content->internal, addr, length);
+
+    if (result && !mrange_contains_addr_inclusive(&content->range, addr))
+    {
+        copy_vmpa(addr, &old);
+        result = false;
+    }
+
+ bad_range:
+
+    return result;
+
+}
+
+
+/******************************************************************************
+*                                                                             *
+*  Paramètres  : content = contenu binaire à venir lire.                      *
+*                addr    = position de la tête de lecture.                    *
 *                length  = quantité d'octets à lire.                          *
 *                                                                             *
 *  Description : Donne accès à une portion des données représentées.          *
diff --git a/src/analysis/disass/area.c b/src/analysis/disass/area.c
index b1d4bb1..c9612af 100644
--- a/src/analysis/disass/area.c
+++ b/src/analysis/disass/area.c
@@ -206,11 +206,9 @@ static bool is_range_blank_in_mem_area_v2(mem_area_v2 *area, phys_t start, phys_
 {
     bool result;                            /* Résultat à renvoyer         */
 
-    if ((start + len) > get_mrange_length(&area->range))
-        result = false;
+    assert((start + len) <= get_mrange_length(&area->range));
 
-    else
-        result = !test_in_bit_field(area->processed, start, len);
+    result = !test_in_bit_field(area->processed, start, len);
 
     return result;
 
diff --git a/src/arch/dalvik/operand.c b/src/arch/dalvik/operand.c
index ac38da5..f0e8c1e 100644
--- a/src/arch/dalvik/operand.c
+++ b/src/arch/dalvik/operand.c
@@ -655,7 +655,7 @@ bool dalvik_read_operands(GArchInstruction *instr, GExeFormat *format, const GBi
         case DALVIK_OPT_20T:
         case DALVIK_OPT_30T:
         case DALVIK_OPT_32X:
-            advance_vmpa(pos, 1);
+            result = g_binary_content_seek(content, pos, 1);
             break;
 
         default:
diff --git a/src/arch/dalvik/processor.c b/src/arch/dalvik/processor.c
index 9fe7253..ab64db7 100644
--- a/src/arch/dalvik/processor.c
+++ b/src/arch/dalvik/processor.c
@@ -536,10 +536,12 @@ static GArchInstruction *g_dalvik_processor_disassemble_pseudo(const GArchProces
     if (low8 != 0x00 /* DOP_NOP */)
         return NULL;
 
+    result = NULL;
+
     copy_vmpa(&tmp, pos);
 
     if (!g_binary_content_read_u8(content, pos, &high8))
-        return NULL;
+        goto gdpdp_exit;
 
     ident = high8 << 8 | low8;
 
@@ -560,7 +562,9 @@ static GArchInstruction *g_dalvik_processor_disassemble_pseudo(const GArchProces
 
     }
 
-    if (result != NULL)
+ gdpdp_exit:
+
+    if (result == NULL)
         copy_vmpa(pos, &tmp);
 
     return result;
diff --git a/src/arch/dalvik/pseudo/fill.c b/src/arch/dalvik/pseudo/fill.c
index 95880fc..e1e1822 100644
--- a/src/arch/dalvik/pseudo/fill.c
+++ b/src/arch/dalvik/pseudo/fill.c
@@ -191,7 +191,10 @@ GArchInstruction *g_dalvik_fill_instr_new(uint16_t ident, const GBinContent *con
 
     consumed = result->array_width * result->array_size;
 
-    advance_vmpa(pos, consumed);
+    if (!g_binary_content_seek(content, pos, consumed))
+        goto gdfin_bad;
+
+    g_arch_instruction_set_displayed_max_length(G_ARCH_INSTRUCTION(result), 8);
 
     return G_ARCH_INSTRUCTION(result);
 
diff --git a/src/arch/dalvik/pseudo/switch.c b/src/arch/dalvik/pseudo/switch.c
index 1bfc124..c1d0982 100644
--- a/src/arch/dalvik/pseudo/switch.c
+++ b/src/arch/dalvik/pseudo/switch.c
@@ -190,7 +190,10 @@ GArchInstruction *g_dalvik_switch_instr_new(uint16_t ident, const GBinContent *c
     else
         consumed = (2 * result->switch_size) * sizeof(uint32_t);
 
-    advance_vmpa(pos, consumed);
+    if (!g_binary_content_seek(content, pos, consumed))
+        goto gdsin_bad;
+
+    g_arch_instruction_set_displayed_max_length(G_ARCH_INSTRUCTION(result), 4);
 
     return G_ARCH_INSTRUCTION(result);
 
diff --git a/src/format/dwarf/v2/form.c b/src/format/dwarf/v2/form.c
index 8a16887..383bb25 100644
--- a/src/format/dwarf/v2/form.c
+++ b/src/format/dwarf/v2/form.c
@@ -197,7 +197,11 @@ bool read_dwarf_v2_form_value(const GDwarfFormat *format, const dw_compil_unit_h
             if (result)
             {
                 copy_vmpa(&iter, get_mrange_addr(&range));
-                advance_vmpa(&iter, offset);
+
+                result = g_binary_content_seek(content, &iter, offset);
+
+                if (!result)
+                    break;
 
                 tmp = g_binary_content_get_raw_access(content, &iter, 1);
                 result = (tmp != NULL);
-- 
cgit v0.11.2-87-g4458