From 3c970a0a1b74a1991be303132221329f3eef0b91 Mon Sep 17 00:00:00 2001 From: Cyrille Bagard 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 + * 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 + * 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