WebAssembly / wabt

The WebAssembly Binary Toolkit
Apache License 2.0
6.87k stars 699 forks source link

wasm-decompile/wabt::Decompiler::DecompileExpr cannot handle when if branch is empty. #1519

Open dwfault opened 4 years ago

dwfault commented 4 years ago

PoC.wasm.zip

Reproduced on git commit commit 2a15fcb, 2020.08.14(latest version).

This is null pointer reference because wasm-decompile cannot handle the situation when if branch is empty. Here's the stack trace:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010001cf9a wasm-decompile`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__is_long(this="") const at string:1420:22
  * frame #1: 0x000000010001cd65 wasm-decompile`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__get_pointer(this="") const at string:1514:17
    frame #2: 0x0000000100004575 wasm-decompile`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::data(this="") const at string:1242:79
    frame #3: 0x000000010000e8d8 wasm-decompile`wabt::string_view::string_view(this=0x0000000104d62920, str="") at string-view.h:247:17
    frame #4: 0x000000010000450d wasm-decompile`wabt::string_view::string_view(this=0x0000000104d62920, str="") at string-view.h:247:44
    frame #5: 0x000000010034c17e wasm-decompile`unsigned long wabt::cat_compute_size<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [3]>(t="", args=<no value available>) [3]) at string-view.h:232:12
    frame #6: 0x000000010034bfae wasm-decompile`unsigned long wabt::cat_compute_size<char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [3]>(t=<no value available>, args="", args=<no value available>) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [3]) at string-view.h:232:36
    frame #7: 0x000000010034bd81 wasm-decompile`unsigned long wabt::cat_compute_size<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [3]>(t="unreachable", args=<no value available>, args="", args=<no value available>) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [3]) at string-view.h:232:36
    frame #8: 0x000000010034b91e wasm-decompile`unsigned long wabt::cat_compute_size<char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [3]>(t=<no value available>, args="unreachable", args=<no value available>, args="", args=<no value available>) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [3]) at string-view.h:232:36
    frame #9: 0x000000010032f6d0 wasm-decompile`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > wabt::cat<char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, char [3]>(args=<no value available>, args="unreachable", args=<no value available>, args="", args=<no value available>) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [5], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const (&) [3]) at string-view.h:238:15
    frame #10: 0x00000001002c121b wasm-decompile`wabt::Decompiler::DecompileExpr(this=0x0000000104f38220, n=0x0000610000001870, parent=0x0000610000001740) at decompiler.cc:509:20
    frame #11: 0x00000001002ba618 wasm-decompile`wabt::Decompiler::DecompileExpr(this=0x0000000104f38220, n=0x0000610000001740, parent=0x0000000000000000) at decompiler.cc:339:22
    frame #12: 0x00000001002aa84a wasm-decompile`wabt::Decompiler::Decompile(this=0x0000000104f38220) at decompiler.cc:797:20
    frame #13: 0x00000001002a612c wasm-decompile`wabt::Decompile(module=0x0000000105322510, options=0x00000001053220c0) at decompiler.cc:826:21
    frame #14: 0x00000001000030c8 wasm-decompile`ProgramMain(argc=2, argv=0x00007ffeefbff6a0) at wasm-decompile.cc:104:18
    frame #15: 0x0000000100004812 wasm-decompile`main(argc=2, argv=0x00007ffeefbff6a0) at wasm-decompile.cc:117:10
    frame #16: 0x00007fff6e695cc9 libdyld.dylib`start + 1

The problem resides in wabt::Decompiler::DecompileExpr:

      case ExprType::If: {
        auto ife = cast<IfExpr>(n.e);
        Value *elsep = nullptr;
        if (!ife->false_.empty()) {
          elsep = &args[2];
        }
        auto& thenp = args[1];
        auto& ifs = args[0];
        bool multiline = ifs.v.size() > 1 || thenp.v.size() > 1;
        size_t width = ifs.width() + thenp.width();
        if (elsep) {
          width += elsep->width();
          multiline = multiline || elsep->v.size() > 1;
        }
        multiline = multiline || width > target_exp_width;
        if (multiline) {
          auto if_start = string_view("if (");
          IndentValue(ifs, if_start.size(), if_start);
          ifs.v.back() += ") {";
          IndentValue(thenp, indent_amount, {});
          std::move(thenp.v.begin(), thenp.v.end(), std::back_inserter(ifs.v));
          if (elsep) {
            ifs.v.push_back("} else {");
            IndentValue(*elsep, indent_amount, {});
            std::move(elsep->v.begin(), elsep->v.end(), std::back_inserter(ifs.v));
          }
          ifs.v.push_back("}");
          ifs.precedence = Precedence::If;
          return std::move(ifs);
        } else {
          auto s = cat("if (", ifs.v[0], ") { ", thenp.v[0], " }"); //---------------->
          if (elsep)
            s += cat(" else { ", elsep->v[0], " }");
          return Value{{std::move(s)}, Precedence::If};
        }
      }

It's pretty obvious. Empty if branch should be allowed as a decompiler. The decompiled code should be as follows.

global g_a:int = 0;

function f_a() {
  let t0, t1 = unreachable, unreachable;
  if (unreachable) {  }
  t0;
  t1;
}

export function et():int {
  return g_a
}
dwfault commented 4 years ago

This bug is found by ADLab of Venustech. Thanks :)