Harden Wasm bytecode parsing. (#304)

Reported by Chris Ertl from Google Security.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Author: PiotrSikora

.bazelrc

@@ -8,37 +8,49 @@ build:clang --action_env=BAZEL_COMPILER=clang
 build:clang --action_env=CC=clang
 build:clang --action_env=CXX=clang++
 
+# Common flags for Clang sanitizers.
+build:clang-xsan --config=clang
+build:clang-xsan --copt -O1
+build:clang-xsan --copt -fno-omit-frame-pointer
+build:clang-xsan --copt -fno-optimize-sibling-calls
+build:clang-xsan --copt -fno-sanitize-recover=all
+build:clang-xsan --linkopt -fsanitize-link-c++-runtime
+build:clang-xsan --linkopt -fuse-ld=lld
+build:clang-xsan --linkopt -rtlib=compiler-rt
+build:clang-xsan --linkopt --unwindlib=libgcc
+
 # Use Clang compiler with Address and Undefined Behavior Sanitizers.
-build:clang-asan --config=clang
+build:clang-asan --config=clang-xsan
 build:clang-asan --copt -DADDRESS_SANITIZER=1
 build:clang-asan --copt -DUNDEFINED_SANITIZER=1
-build:clang-asan --copt -O1
-build:clang-asan --copt -fno-omit-frame-pointer
-build:clang-asan --copt -fno-optimize-sibling-calls
 build:clang-asan --copt -fsanitize=address,undefined
 build:clang-asan --copt -fsanitize-address-use-after-scope
 build:clang-asan --linkopt -fsanitize=address,undefined
 build:clang-asan --linkopt -fsanitize-address-use-after-scope
-build:clang-asan --linkopt -fsanitize-link-c++-runtime
-build:clang-asan --linkopt -fuse-ld=lld
 build:clang-asan --test_env=ASAN_OPTIONS=check_initialization_order=1:detect_stack_use_after_return=1:strict_init_order=1:strict_string_checks=1
-build:clang-asan --test_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1
+build:clang-asan --test_env=UBSAN_OPTIONS=print_stacktrace=1
 build:clang-asan --test_env=ASAN_SYMBOLIZER_PATH
 
 # Use Clang compiler with Address and Undefined Behavior Sanitizers (strict version).
 build:clang-asan-strict --config=clang-asan
-build:clang-asan-strict --copt -fsanitize=integer
-build:clang-asan-strict --linkopt -fsanitize=integer
+build:clang-asan-strict --copt -fsanitize=integer,local-bounds,nullability
+build:clang-asan-strict --linkopt -fsanitize=integer,local-bounds,nullability
+
+# Use Honggfuzz with Address and Undefined Behavior Sanitizers (strict version).
+build:clang-asan-honggfuzz --config=clang-asan-strict
+build:clang-asan-honggfuzz --@rules_fuzzing//fuzzing:cc_engine=@rules_fuzzing//fuzzing/engines:honggfuzz
+build:clang-asan-honggfuzz --@rules_fuzzing//fuzzing:cc_engine_instrumentation=honggfuzz
+
+# Use LibFuzzer with Address and Undefined Behavior Sanitizers (strict version).
+build:clang-asan-libfuzzer --config=clang-asan-strict
+build:clang-asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine=@rules_fuzzing//fuzzing/engines:libfuzzer
+build:clang-asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_instrumentation=libfuzzer
 
 # Use Clang compiler with Thread Sanitizer.
-build:clang-tsan --config=clang
+build:clang-tsan --config=clang-xsan
 build:clang-tsan --copt -DTHREAD_SANITIZER=1
-build:clang-tsan --copt -O1
-build:clang-tsan --copt -fno-omit-frame-pointer
-build:clang-tsan --copt -fno-optimize-sibling-calls
 build:clang-tsan --copt -fsanitize=thread
 build:clang-tsan --linkopt -fsanitize=thread
-build:clang-tsan --linkopt -fuse-ld=lld
 
 # Use Clang-Tidy tool.
 build:clang-tidy --aspects @bazel_clang_tidy//clang_tidy:clang_tidy.bzl%clang_tidy_aspect

.github/workflows/test.yml

@@ -125,6 +125,7 @@ jobs:
           os: windows-2019
           arch: x86_64
           action: test
+          targets: -//test/fuzz/...
         - name: 'V8 on Linux/x86_64'
           engine: 'v8'
           repo: 'v8'
@@ -155,6 +156,7 @@ jobs:
           os: ubuntu-20.04
           arch: aarch64
           action: test
+          targets: -//test/fuzz/...
           flags: --config=zig-cc-linux-aarch64 --@v8//bazel/config:v8_target_cpu=arm64
           deps: qemu-user-static libc6-arm64-cross
           cache: true
@@ -228,6 +230,7 @@ jobs:
           os: windows-2019
           arch: x86_64
           action: test
+          targets: -//test/fuzz/...
         - name: 'WAVM on Linux/x86_64'
           engine: 'wavm'
           repo: 'com_github_wavm_wavm'
@@ -280,14 +283,15 @@ jobs:
         done
 
     - name: Bazel build/test
+      shell: bash
       run: >
         ${{ matrix.run_under }}
         bazel ${{ matrix.action }}
         --verbose_failures
         --test_output=errors
         --define engine=${{ matrix.engine }}
         ${{ matrix.flags }}
-        //test/...
+        -- //test/... ${{ matrix.targets }}
 
     - name: Bazel build/test (signed Wasm module)
       if: ${{ matrix.engine != 'null' && !startsWith(matrix.os, 'windows') }}

bazel/dependencies.bzl

@@ -17,6 +17,8 @@ load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
 load("@proxy_wasm_cpp_host//bazel/cargo/wasmsign:crates.bzl", "wasmsign_fetch_remote_crates")
 load("@proxy_wasm_cpp_host//bazel/cargo/wasmtime:crates.bzl", "wasmtime_fetch_remote_crates")
 load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")
+load("@rules_fuzzing//fuzzing:init.bzl", "rules_fuzzing_init")
+load("@rules_fuzzing//fuzzing:repositories.bzl", "rules_fuzzing_dependencies")
 load("@rules_python//python:pip.bzl", "pip_install")
 load("@rules_rust//rust:repositories.bzl", "rust_repositories", "rust_repository_set")
 
@@ -25,6 +27,9 @@ def proxy_wasm_cpp_host_dependencies():
 
     rules_foreign_cc_dependencies()
 
+    rules_fuzzing_dependencies()
+    rules_fuzzing_init()
+
     rust_repositories()
     rust_repository_set(
         name = "rust_linux_x86_64",

bazel/repositories.bzl

@@ -55,6 +55,14 @@ def proxy_wasm_cpp_host_repositories():
         url = "https://github.com/bazelbuild/rules_foreign_cc/archive/0.7.1.tar.gz",
     )
 
+    maybe(
+        http_archive,
+        name = "rules_fuzzing",
+        sha256 = "23bb074064c6f488d12044934ab1b0631e8e6898d5cf2f6bde087adb01111573",
+        strip_prefix = "rules_fuzzing-0.3.1",
+        url = "https://github.com/bazelbuild/rules_fuzzing/archive/v0.3.1.zip",
+    )
+
     maybe(
         http_archive,
         name = "rules_python",

src/bytecode_util.cc

@@ -48,15 +48,18 @@ bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersi
       return false;
     }
     if (section_type == 7 /* export section */) {
+      const char *section_end = pos + section_len;
       uint32_t export_vector_size = 0;
-      if (!parseVarint(pos, end, export_vector_size) || pos + export_vector_size > end) {
+      if (!parseVarint(pos, section_end, export_vector_size) ||
+          pos + export_vector_size > section_end) {
         return false;
       }
       // Search thourgh exports.
       for (uint32_t i = 0; i < export_vector_size; i++) {
         // Parse name of the export.
         uint32_t export_name_size = 0;
-        if (!parseVarint(pos, end, export_name_size) || pos + export_name_size > end) {
+        if (!parseVarint(pos, section_end, export_name_size) ||
+            pos + export_name_size > section_end) {
           return false;
         }
         const auto *const name_begin = pos;
@@ -65,7 +68,7 @@ bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersi
           return false;
         }
         // Check if it is a function type export
-        if (*pos++ == 0x00) {
+        if (*pos++ == 0x00 /* function */) {
           const std::string export_name = {name_begin, export_name_size};
           // Check the name of the function.
           if (export_name == "proxy_abi_version_0_1_0") {
@@ -114,24 +117,25 @@ bool BytecodeUtil::getCustomSection(std::string_view bytecode, std::string_view
     }
     if (section_type == 0) {
       // Custom section.
-      const auto *const section_data_start = pos;
+      const char *section_end = pos + section_len;
       uint32_t section_name_len = 0;
-      if (!BytecodeUtil::parseVarint(pos, end, section_name_len) || pos + section_name_len > end) {
+      if (!BytecodeUtil::parseVarint(pos, section_end, section_name_len) ||
+          pos + section_name_len > section_end) {
         return false;
       }
       if (section_name_len == name.size() && ::memcmp(pos, name.data(), section_name_len) == 0) {
         pos += section_name_len;
-        ret = {pos, static_cast<size_t>(section_data_start + section_len - pos)};
+        ret = {pos, static_cast<size_t>(section_end - pos)};
         return true;
       }
-      pos = section_data_start + section_len;
+      pos = section_end;
     } else {
       // Skip other sections.
       pos += section_len;
     }
   }
   return true;
-};
+}
 
 bool BytecodeUtil::getFunctionNameIndex(std::string_view bytecode,
                                         std::unordered_map<uint32_t, std::string> &ret) {
@@ -242,16 +246,32 @@ bool BytecodeUtil::getStrippedSource(std::string_view bytecode, std::string &ret
 
 bool BytecodeUtil::parseVarint(const char *&pos, const char *end, uint32_t &ret) {
   uint32_t shift = 0;
+  uint32_t total = 0;
+  uint32_t v;
   char b;
-  do {
+  while (pos < end) {
     if (pos + 1 > end) {
+      // overread
       return false;
     }
     b = *pos++;
-    ret += (b & 0x7f) << shift;
+    v = (b & 0x7f);
+    if (shift == 28 && v > 3) {
+      // overflow
+      return false;
+    }
+    total += v << shift;
+    if ((b & 0x80) == 0) {
+      ret = total;
+      return true;
+    }
     shift += 7;
-  } while ((b & 0x80) != 0);
-  return ret != static_cast<uint32_t>(-1);
+    if (shift > 28) {
+      // overflow
+      return false;
+    }
+  }
+  return false;
 }
 
 } // namespace proxy_wasm

test/fuzz/BUILD

@@ -0,0 +1,19 @@
+load("@rules_fuzzing//fuzzing:cc_defs.bzl", "cc_fuzz_test")
+
+licenses(["notice"])  # Apache 2
+
+package(default_visibility = ["//visibility:public"])
+
+filegroup(
+    name = "corpus_bytecode",
+    srcs = glob(["corpus_bytecode/**"]),
+)
+
+cc_fuzz_test(
+    name = "bytecode_util_fuzzer",
+    srcs = ["bytecode_util_fuzzer.cc"],
+    corpus = [":corpus_bytecode"],
+    deps = [
+        "//:lib",
+    ],
+)

test/fuzz/bytecode_util_fuzzer.cc

@@ -0,0 +1,41 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "include/proxy-wasm/bytecode_util.h"
+
+#include <string>
+
+namespace proxy_wasm {
+namespace {
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+  auto bytecode = std::string_view(reinterpret_cast<const char *>(data), size);
+
+  AbiVersion version;
+  BytecodeUtil::getAbiVersion(bytecode, version);
+
+  std::string_view custom_section;
+  BytecodeUtil::getCustomSection(bytecode, "precompiled", custom_section);
+
+  std::string stripped_source;
+  BytecodeUtil::getStrippedSource(bytecode, stripped_source);
+
+  std::unordered_map<uint32_t, std::string> function_names;
+  BytecodeUtil::getFunctionNameIndex(bytecode, function_names);
+
+  return 0;
+}
+
+} // namespace
+} // namespace proxy_wasm