From 7fc86b4051b91adbd0b99f41c19d866ae0760586 Mon Sep 17 00:00:00 2001
From: Cyrille Bagard <nocbos@gmail.com>
Date: Mon, 11 Feb 2019 01:52:04 +0100
Subject: Improved the loop detection.

---
 src/analysis/disass/loop.c         |  55 +++++++++++++++++++++++++++++++------
 tests/analysis/disass/block.py     |  53 +++++++++++++++++++++++++++++++++--
 tests/analysis/disass/sub_a1bc.bin | Bin 0 -> 416 bytes
 3 files changed, 96 insertions(+), 12 deletions(-)
 create mode 100644 tests/analysis/disass/sub_a1bc.bin

diff --git a/src/analysis/disass/loop.c b/src/analysis/disass/loop.c
index b001992..c35bab7 100644
--- a/src/analysis/disass/loop.c
+++ b/src/analysis/disass/loop.c
@@ -78,6 +78,9 @@ static void tag_loop_head(bblock_info_t *, bblock_info_t *);
 static bblock_info_t *traverse_basic_blocks_dfs(bblock_info_t *, GBlockList *, bblock_info_t *, unsigned int);
 
 /* Indique si une boucle doit être définie. */
+static bool _should_be_natural_loop_link(bblock_info_t *, bblock_info_t *);
+
+/* Indique si une boucle doit être définie. */
 static bool should_be_natural_loop_link(bblock_info_t *, bblock_info_t *);
 
 /* Définit les boucles entre un ensemble de blocs basiques. */
@@ -356,7 +359,7 @@ static bblock_info_t *traverse_basic_blocks_dfs(bblock_info_t *root, GBlockList
 *                                                                             *
 ******************************************************************************/
 
-static bool should_be_natural_loop_link(bblock_info_t *dest, bblock_info_t *header)
+static bool _should_be_natural_loop_link(bblock_info_t *dest, bblock_info_t *header)
 {
     bool result;                            /* Conclusion à retourner      */
 
@@ -372,6 +375,33 @@ static bool should_be_natural_loop_link(bblock_info_t *dest, bblock_info_t *head
 
 /******************************************************************************
 *                                                                             *
+*  Paramètres  : dest   = informations du bloc de destination.                *
+*                header = informations de l'entête de boucle.                 *
+*                                                                             *
+*  Description : Indique si une boucle doit être définie.                     *
+*                                                                             *
+*  Retour      : true si une boucle naturelle est bien présente.              *
+*                                                                             *
+*  Remarques   : -                                                            *
+*                                                                             *
+******************************************************************************/
+
+static bool should_be_natural_loop_link(bblock_info_t *dest, bblock_info_t *header)
+{
+    bool result;                            /* Conclusion à retourner      */
+
+    result = _should_be_natural_loop_link(dest, header);
+
+    if (!result && dest->iloop_header != NULL)
+        result = should_be_natural_loop_link(dest->iloop_header, header);
+
+    return result;
+
+}
+
+
+/******************************************************************************
+*                                                                             *
 *  Paramètres  : list = liste de blocs de code à consulter.                   *
 *                info = informations complémentaires quant aux blocs.         *
 *                                                                             *
@@ -445,7 +475,7 @@ static void define_basic_blocks_loops(GBlockList *list, bblock_info_t *info)
             links = get_block_successors(block, info, &count);
 
             for (k = 0; k < count; k++)
-                if (should_be_natural_loop_link(links[k].info, iter->iloop_header)
+                if (_should_be_natural_loop_link(links[k].info, iter->iloop_header)
                     /**
                      * Il se peut qu'un bloc fasse référence à lui même !
                      *
@@ -504,16 +534,23 @@ void detect_loops_in_basic_blocks(GBlockList *list)
 
     count = g_block_list_count_blocks(list);
 
-    if (count > 1)
-    {
-        info = calloc(count, sizeof(bblock_info_t));
+    /**
+     * Un premier jet consistait à filtrer sur le nombre de blocs : s'il n'y
+     * en avait qu'un, il n'y avait à priori pas de raison de rechercher des
+     * boucles !
+     *
+     * Mais c'était sans compter une routine se résumant à une boucle infinie...
+     *
+     * C'est par exemple le cas avec la fonction operator new[] (_ZnajRKSt9nothrow_t)
+     * de l'échantillon b6990fc6913d839809c72d1d482cb2c295c4840fc6a1f40f38923464e958ffae.
+     */
 
-        traverse_basic_blocks_dfs(&info[0], list, info, 1);
+    info = calloc(count, sizeof(bblock_info_t));
 
-        define_basic_blocks_loops(list, info);
+    traverse_basic_blocks_dfs(&info[0], list, info, 1);
 
-        free(info);
+    define_basic_blocks_loops(list, info);
 
-    }
+    free(info);
 
 }
diff --git a/tests/analysis/disass/block.py b/tests/analysis/disass/block.py
index 9b9d529..0907542 100644
--- a/tests/analysis/disass/block.py
+++ b/tests/analysis/disass/block.py
@@ -192,14 +192,14 @@ class TestBasicBlocks(ChrysalideTestCase):
     def testOtherLoops(self):
         """Check situation with some binary codes old troubles."""
 
-        # Malwre e8e1bc048ef123a9757a9b27d1bf53c092352a26bdbf9fbdc10109415b5cadac
-        # Fonction jinit_color_converter de lib/armeabi/libgame.so
-
         fullname = sys.modules[self.__class__.__module__].__file__
         filename = os.path.basename(fullname)
 
         baselen = len(fullname) - len(filename)
 
+        # Malware e8e1bc048ef123a9757a9b27d1bf53c092352a26bdbf9fbdc10109415b5cadac
+        # Fonction jinit_color_converter de lib/armeabi/libgame.so
+
         cnt = FileContent(fullname[:baselen] + 'jinit_color_converter.bin')
         self.assertIsNotNone(cnt)
 
@@ -243,3 +243,50 @@ class TestBasicBlocks(ChrysalideTestCase):
                     loop_count += 1
 
         self.assertEqual(loop_count, 3)
+
+        # Malware 6e4b64ede44bf4cfb36da04aacc9a22ba73e11be2deac339e275d3bde3b31311
+        # Fonction sub_a1bc de lib/armeabi-v7a/liblamelib.so
+
+        cnt = FileContent(fullname[:baselen] + 'sub_a1bc.bin')
+        self.assertIsNotNone(cnt)
+
+        fmt = FlatFormat(cnt)
+
+        fmt.set_machine('armv7')
+
+        base = vmpa(0, 0xa1bc)
+
+        p = BinPortion(BinPortion.BPC_CODE, base, cnt.size)
+        p.rights = BinPortion.PAC_READ | BinPortion.PAC_EXEC
+
+        fmt.register_user_portion(p)
+
+        fmt.register_code_point(base.virt + 1, True)
+
+        sym = BinRoutine()
+        sym.range = p.range
+
+        fmt.add_symbol(sym)
+
+        binary = LoadedBinary(fmt)
+
+        status = binary.analyze_and_wait()
+        self.assertTrue(status)
+
+        loop_count = 0
+
+        for blk in sym.basic_blocks:
+            for _, dt in blk.destinations:
+                if dt == ArchInstruction.ILT_LOOP:
+                    loop_count += 1
+
+        self.assertEqual(loop_count, 8)
+
+        loop_count = 0
+
+        for ins in binary.processor.instrs:
+            for _, dt in ins.destinations:
+                if dt == ArchInstruction.ILT_LOOP:
+                    loop_count += 1
+
+        self.assertEqual(loop_count, 8)
diff --git a/tests/analysis/disass/sub_a1bc.bin b/tests/analysis/disass/sub_a1bc.bin
new file mode 100644
index 0000000..dc18852
Binary files /dev/null and b/tests/analysis/disass/sub_a1bc.bin differ
-- 
cgit v0.11.2-87-g4458