[Bug libasm/25068] Several crashes inside libasm

["mark at klomp dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=25068

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2019-10-16
                 CC|                            |mark at klomp dot org
     Ever confirmed|0                           |1

--- Comment #1 from Mark Wielaard <mark at klomp dot org> ---
Thanks for the example crashes. These seem caused by 2 asserts, a missing
bounds check and an off-by-one bounds check. Which should be resolved by the
following patch:

diff --git a/libcpu/i386_data.h b/libcpu/i386_data.h
index b8a34c3e..06356b8a 100644
--- a/libcpu/i386_data.h
+++ b/libcpu/i386_data.h
@@ -1336,7 +1336,7 @@ FCT_sel (struct output_data *d)
 {
   assert (d->opoff1 % 8 == 0);
   assert (d->opoff1 / 8 == 5);
-  if (*d->param_start + 2 > d->end)
+  if (*d->param_start + 2 >= d->end)
     return -1;
   *d->param_start += 2;
   uint16_t absval = read_2ubyte_unaligned (&d->data[5]);
diff --git a/libcpu/i386_disasm.c b/libcpu/i386_disasm.c
index 8a206398..4422ffa2 100644
--- a/libcpu/i386_disasm.c
+++ b/libcpu/i386_disasm.c
@@ -610,7 +610,9 @@ i386_disasm (Ebl *ebl __attribute__((unused)),

                  /* Account for displacement.  */
                  if ((modrm & 0xc7) == 5 || (modrm & 0xc0) == 0x80
-                     || ((modrm & 0xc7) == 0x4 && (codep[0] & 0x7) == 0x5))
+                     || ((modrm & 0xc7) == 0x4
+                         && param_start < end
+                         && (codep[0] & 0x7) == 0x5))
                    param_start += 4;
                  else if ((modrm & 0xc0) == 0x40)
                    param_start += 1;
@@ -821,7 +823,8 @@ i386_disasm (Ebl *ebl __attribute__((unused)),
                            }
                          FALLTHROUGH;
                        default:
-                         assert (! "INVALID not handled");
+                         str = "INVALID not handled";
+                         break;
                        }
                    }
                  else
@@ -1124,8 +1127,9 @@ i386_disasm (Ebl *ebl __attribute__((unused)),
       /* Invalid (or at least unhandled) opcode.  */
       if (prefixes != 0)
        goto print_prefix;
-      assert (*startp == data);
-      ++data;
+      /* Make sure we get past the unrecognized opcode if we haven't yet.  */
+      if (*startp == data)
+       ++data;
       ADD_STRING ("(bad)");
       addr += data - begin;

-- 
You are receiving this mail because:
You are on the CC list for the bug.