ubsan: wasm32: signed integer overflow

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

ubsan: wasm32: signed integer overflow

Alan Modra-3
The signed integer overflow occurred when adding one to target_count
  for (i = 0; i < target_count + 1; i++)
but that's the least of the worries here.  target_count was long and i
int, leading to the possibility of a loop that never ended.

So to avoid this type of vulnerability, this patch uses what I believe
to be the proper types for arguments of various wasm32 opcodes, rather
than using "long" which may change in size.

gas/
        * testsuite/gas/wasm32/allinsn.d: Update expected output.
opcodes/
        * wasm32-dis.c (print_insn_wasm32): Localise variables.  Store
        result of wasm_read_leb128 in a uint64_t and check that bits
        are not lost when copying to other locals.  Use uint32_t for
        most locals.  Use PRId64 when printing int64_t.

diff --git a/gas/testsuite/gas/wasm32/allinsn.d b/gas/testsuite/gas/wasm32/allinsn.d
index c594c72501..4429385386 100644
--- a/gas/testsuite/gas/wasm32/allinsn.d
+++ b/gas/testsuite/gas/wasm32/allinsn.d
@@ -20,7 +20,7 @@ Disassembly of section .text:
   12: 8b           f32.abs
   13: 92           f32.add
   14: 8d           f32.ceil
-  15: 43 d0 0f 49 f32.const 3.141590118408203125
+  15: 43 d0 0f 49 f32.const 3.14159012
   19: 40
   1a: b2           f32.convert_s/i32
   1b: b4           f32.convert_s/i64
@@ -50,7 +50,7 @@ Disassembly of section .text:
   37: 99           f64.abs
   38: a0           f64.add
   39: 9b           f64.ceil
-  3a: 44 97 5f 4f f64.const 3.14158999999999976088e\+200
+  3a: 44 97 5f 4f f64.const 3.1415899999999998e\+200
   3e: fd bc 6a 90
   42: 69
   43: b7           f64.convert_s/i32
diff --git a/opcodes/wasm32-dis.c b/opcodes/wasm32-dis.c
index aea93b81c2..946ce5fb68 100644
--- a/opcodes/wasm32-dis.c
+++ b/opcodes/wasm32-dis.c
@@ -271,33 +271,10 @@ print_insn_wasm32 (bfd_vma pc, struct disassemble_info *info)
   void *stream = info->stream;
   fprintf_ftype prin = info->fprintf_func;
   struct wasm32_private_data *private_data = info->private_data;
-  long long constant = 0;
-  double fconstant = 0.0;
-  long flags = 0;
-  long offset = 0;
-  long depth = 0;
-  long function_index = 0;
-  long target_count = 0;
-  long block_type = 0;
-  int len = 1;
-  int ret = 0;
-  unsigned int bytes_read = 0;
-  int i;
-  const char *locals[] =
-    {
-      "$dpc", "$sp1", "$r0", "$r1", "$rpc", "$pc0",
-      "$rp", "$fp", "$sp",
-      "$r2", "$r3", "$r4", "$r5", "$r6", "$r7",
-      "$i0", "$i1", "$i2", "$i3", "$i4", "$i5", "$i6", "$i7",
-      "$f0", "$f1", "$f2", "$f3", "$f4", "$f5", "$f6", "$f7",
-    };
-  int nlocals = ARRAY_SIZE (locals);
-  const char *globals[] =
-    {
-      "$got", "$plt", "$gpo"
-    };
-  int nglobals = ARRAY_SIZE (globals);
-  bfd_boolean error = FALSE;
+  uint64_t val;
+  int len;
+  unsigned int bytes_read;
+  bfd_boolean error;
 
   if (info->read_memory_func (pc, buffer, 1, info))
     return -1;
@@ -313,189 +290,239 @@ print_insn_wasm32 (bfd_vma pc, struct disassemble_info *info)
       prin (stream, "\t.byte 0x%02x\n", buffer[0]);
       return 1;
     }
-  else
+
+  len = 1;
+
+  prin (stream, "\t");
+  prin (stream, "%s", op->name);
+
+  if (op->clas == wasm_typed)
     {
-      len = 1;
-
-      prin (stream, "\t");
-      prin (stream, "%s", op->name);
-
-      if (op->clas == wasm_typed)
-        {
-          block_type = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          switch (block_type)
-            {
-            case BLOCK_TYPE_NONE:
-              prin (stream, "[]");
-              break;
-            case BLOCK_TYPE_I32:
-              prin (stream, "[i]");
-              break;
-            case BLOCK_TYPE_I64:
-              prin (stream, "[l]");
-              break;
-            case BLOCK_TYPE_F32:
-              prin (stream, "[f]");
-              break;
-            case BLOCK_TYPE_F64:
-              prin (stream, "[d]");
-              break;
-            }
-        }
-
-      switch (op->clas)
-        {
-        case wasm_special:
-        case wasm_eqz:
-        case wasm_binary:
-        case wasm_unary:
-        case wasm_conv:
-        case wasm_relational:
-        case wasm_drop:
-        case wasm_signature:
-        case wasm_call_import:
-        case wasm_typed:
-        case wasm_select:
-          break;
-
-        case wasm_break_table:
-          target_count = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %ld", target_count);
-          for (i = 0; i < target_count + 1; i++)
-            {
-              long target = 0;
-              target = wasm_read_leb128
-                (pc + len, info, &error, &bytes_read, FALSE);
-              if (error)
-                return -1;
-              len += bytes_read;
-              prin (stream, " %ld", target);
-            }
-          break;
-
-        case wasm_break:
-        case wasm_break_if:
-          depth = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %ld", depth);
-          break;
-
-        case wasm_return:
-          break;
-
-        case wasm_constant_i32:
-        case wasm_constant_i64:
-          constant = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, TRUE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %lld", constant);
-          break;
-
-        case wasm_constant_f32:
-          /* This appears to be the best we can do, even though we're
-             using host doubles for WebAssembly floats.  */
-          ret = read_f32 (&fconstant, pc + len, info);
-          if (ret < 0)
-            return -1;
-          len += ret;
-  prin (stream, " %.9g", fconstant);
-          break;
-
-        case wasm_constant_f64:
-          ret = read_f64 (&fconstant, pc + len, info);
-          if (ret < 0)
-            return -1;
-          len += ret;
-  prin (stream, " %.17g", fconstant);
-          break;
-
-        case wasm_call:
-          function_index = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " ");
-          private_data->section_prefix = ".space.function_index";
-          (*info->print_address_func) ((bfd_vma) function_index, info);
-          private_data->section_prefix = NULL;
-          break;
-
-        case wasm_call_indirect:
-          constant = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %lld", constant);
-          constant = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %lld", constant);
-          break;
-
-        case wasm_get_local:
-        case wasm_set_local:
-        case wasm_tee_local:
-          constant = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %lld", constant);
-          if (strcmp (op->name + 4, "local") == 0)
-            {
-              if (private_data->print_registers
-                  && constant >= 0 && constant < nlocals)
-                prin (stream, " <%s>", locals[constant]);
-            }
-          else
-            {
-              if (private_data->print_well_known_globals
-                  && constant >= 0 && constant < nglobals)
-                prin (stream, " <%s>", globals[constant]);
-            }
-          break;
-
-        case wasm_grow_memory:
-        case wasm_current_memory:
-          constant = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " %lld", constant);
-          break;
-
-        case wasm_load:
-        case wasm_store:
-          flags = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          offset = wasm_read_leb128
-            (pc + len, info, &error, &bytes_read, FALSE);
-          if (error)
-            return -1;
-          len += bytes_read;
-          prin (stream, " a=%ld %ld", flags, offset);
-        }
+      val = wasm_read_leb128 (pc + len, info, &error, &bytes_read, FALSE);
+      if (error)
+ return -1;
+      len += bytes_read;
+      switch (val)
+ {
+ case BLOCK_TYPE_NONE:
+  prin (stream, "[]");
+  break;
+ case BLOCK_TYPE_I32:
+  prin (stream, "[i]");
+  break;
+ case BLOCK_TYPE_I64:
+  prin (stream, "[l]");
+  break;
+ case BLOCK_TYPE_F32:
+  prin (stream, "[f]");
+  break;
+ case BLOCK_TYPE_F64:
+  prin (stream, "[d]");
+  break;
+ default:
+  return -1;
+ }
+    }
+
+  switch (op->clas)
+    {
+    case wasm_special:
+    case wasm_eqz:
+    case wasm_binary:
+    case wasm_unary:
+    case wasm_conv:
+    case wasm_relational:
+    case wasm_drop:
+    case wasm_signature:
+    case wasm_call_import:
+    case wasm_typed:
+    case wasm_select:
+      break;
+
+    case wasm_break_table:
+      {
+ uint32_t target_count, i;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ target_count = val;
+ if (error || target_count != val || target_count == (uint32_t) -1)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", target_count);
+ for (i = 0; i < target_count + 1; i++)
+  {
+    uint32_t target;
+    val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+    FALSE);
+    target = val;
+    if (error || target != val)
+      return -1;
+    len += bytes_read;
+    prin (stream, " %u", target);
+  }
+      }
+      break;
+
+    case wasm_break:
+    case wasm_break_if:
+      {
+ uint32_t depth;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ depth = val;
+ if (error || depth != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", depth);
+      }
+      break;
+
+    case wasm_return:
+      break;
+
+    case wasm_constant_i32:
+    case wasm_constant_i64:
+      val = wasm_read_leb128 (pc + len, info, &error, &bytes_read, TRUE);
+      if (error)
+ return -1;
+      len += bytes_read;
+      prin (stream, " %" PRId64, val);
+      break;
+
+    case wasm_constant_f32:
+      {
+ double fconstant;
+ int ret;
+ /* This appears to be the best we can do, even though we're
+   using host doubles for WebAssembly floats.  */
+ ret = read_f32 (&fconstant, pc + len, info);
+ if (ret < 0)
+  return -1;
+ len += ret;
+ prin (stream, " %.9g", fconstant);
+      }
+      break;
+
+    case wasm_constant_f64:
+      {
+ double fconstant;
+ int ret;
+ ret = read_f64 (&fconstant, pc + len, info);
+ if (ret < 0)
+  return -1;
+ len += ret;
+ prin (stream, " %.17g", fconstant);
+      }
+      break;
+
+    case wasm_call:
+      {
+ uint32_t function_index;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ function_index = val;
+ if (error || function_index != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " ");
+ private_data->section_prefix = ".space.function_index";
+ (*info->print_address_func) ((bfd_vma) function_index, info);
+ private_data->section_prefix = NULL;
+      }
+      break;
+
+    case wasm_call_indirect:
+      {
+ uint32_t type_index, xtra_index;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ type_index = val;
+ if (error || type_index != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", type_index);
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ xtra_index = val;
+ if (error || xtra_index != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", xtra_index);
+      }
+      break;
+
+    case wasm_get_local:
+    case wasm_set_local:
+    case wasm_tee_local:
+      {
+ uint32_t local_index;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ local_index = val;
+ if (error || local_index != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", local_index);
+ if (strcmp (op->name + 4, "local") == 0)
+  {
+    static const char *locals[] =
+      {
+ "$dpc", "$sp1", "$r0", "$r1", "$rpc", "$pc0",
+ "$rp", "$fp", "$sp",
+ "$r2", "$r3", "$r4", "$r5", "$r6", "$r7",
+ "$i0", "$i1", "$i2", "$i3", "$i4", "$i5", "$i6", "$i7",
+ "$f0", "$f1", "$f2", "$f3", "$f4", "$f5", "$f6", "$f7",
+      };
+    if (private_data->print_registers
+ && local_index < ARRAY_SIZE (locals))
+      prin (stream, " <%s>", locals[local_index]);
+  }
+ else
+  {
+    static const char *globals[] =
+      {
+ "$got", "$plt", "$gpo"
+      };
+    if (private_data->print_well_known_globals
+ && local_index < ARRAY_SIZE (globals))
+      prin (stream, " <%s>", globals[local_index]);
+  }
+      }
+      break;
+
+    case wasm_grow_memory:
+    case wasm_current_memory:
+      {
+ uint32_t reserved_size;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ reserved_size = val;
+ if (error || reserved_size != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " %u", reserved_size);
+      }
+      break;
+
+    case wasm_load:
+    case wasm_store:
+      {
+ uint32_t flags, offset;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ flags = val;
+ if (error || flags != val)
+  return -1;
+ len += bytes_read;
+ val = wasm_read_leb128 (pc + len, info, &error, &bytes_read,
+ FALSE);
+ offset = val;
+ if (error || offset != val)
+  return -1;
+ len += bytes_read;
+ prin (stream, " a=%u %u", flags, offset);
+      }
+      break;
     }
   return len;
 }

--
Alan Modra
Australia Development Lab, IBM