`Parser#parse` throws all kinds of exceptions for malformed input

[Marcono1234]

Version

1.0.1

Description

Out of curiosity I used the JVM fuzzing library Jazzer on Parser#parse to see how it handles malformed input.

As part of this I encountered the following exceptions (other than WasmEngineException):

• IllegalArgumentException
• IndexOutOfBoundsException
• ArrayIndexOutOfBoundsException
• BufferUnderflowException
• ArithmeticException
• NullPointerException
• NegativeArraySizeException
• OutOfMemoryError
• AssertionError

I assume it would be good to fix the AssertionError (i.e., here throw an exception instead of using an assert).
OutOfMemoryError occurred for cases where the parser eagerly allocated user-defined amount of memory. That might be acceptable to still keep good performance?

This probably has no security impact though (not even the OutOfMemoryError)? If the user was parsing untrusted Wasm code, they most likely would execute it afterwards and that would be the main security risk then, and could suffer from denial-of-service attacks as well.

Do you think that current Parser behavior is acceptable to avoid adding checks in multiple places, for Wasm code which is most likely malformed anyway? Or should Parser only throw WasmEngineException?

Jazzer setup (click to expand)

Dependency:

<dependency>
  <groupId>com.code-intelligence</groupId>
  <artifactId>jazzer-junit</artifactId>
  <version>0.30.0</version>
  <scope>test</scope>
</dependency>

junit-platform.properties:

jazzer.instrumentation_includes=run.endive.**
jazzer.instrumentation_excludes=run.endive.$gen.**

# Workaround JUnit timeout causing spurious test failure, see https://github.com/CodeIntelligenceTesting/jazzer/issues/1060
junit.jupiter.execution.timeout.mode=disabled

Test class ParserTest:

import com.code_intelligence.jazzer.junit.FuzzTest;
import com.code_intelligence.jazzer.mutation.annotation.NotNull;
import com.code_intelligence.jazzer.mutation.annotation.ValuePool;
import org.junit.jupiter.api.Timeout;
import run.endive.runtime.Instance;
import run.endive.wasm.Parser;
import run.endive.wasm.WasmEngineException;
import run.endive.wasm.WasmModule;

import java.util.concurrent.TimeUnit;

public class ParserTest {
    // Corpus from https://github.com/bytecodealliance/endive/tree/75057984f851fd2b13fb04db42f66ab5600585ae/wasm-corpus/src/main/resources/compiled
    private static final String CORPUS_PATH = "src/test/resources/endive-wasm-corpus/**";

    @Timeout(value = 10, unit = TimeUnit.SECONDS)
    @FuzzTest
    void parse(byte[] bytes) {
        WasmModule module;
        try {
            module = Parser.parse(bytes);
        } catch (WasmEngineException e) {
            return;
        }

        try {
            Instance instance = Instance.builder(module).build();
        }  catch (WasmEngineException e) {
            return;
        }
    }

    @Timeout(value = 10, unit = TimeUnit.SECONDS)
    @FuzzTest
    // `ignored` parameter as workaround for https://github.com/CodeIntelligenceTesting/jazzer/issues/1022
    void parseWithCorpus(byte @ValuePool(files = CORPUS_PATH) @NotNull [] bytes, boolean ignored) {
        WasmModule module;
        try {
            module = Parser.parse(bytes);
        } catch (WasmEngineException e) {
            return;
        }

        try {
            Instance instance = Instance.builder(module).build();
        }  catch (WasmEngineException e) {
            return;
        }
    }
}

Then manually run the individual test methods (running multiple is not possible yet) for example in your IDE, and edit the IDE run configuration to set the environment variable JAZZER_FUZZ=1.

[Marcono1234]

Have also been fuzzing the runtime compiler and the interpreter a bit now. Both also seem to throw exceptions other than WasmEngineException and WasmException:

• for the runtime compiler that might be acceptable because the documentation discourages using it for untrusted code?
• some exceptions are simply because Endive's internal code has throw new RuntimeException and throw new IllegalArgumentException (where throw new WasmEngineException might be better?)
• exceptions such as NegativeArraySizeException, ArrayIndexOutOfBoundsException and NoSuchElementException occur because of missing explicit validation
  Maybe that is acceptable to avoid the overhead and verbosity of explicit bounds checks?

One solution could of course also be to have somewhere in Endive before compiling or executing the Wasm code a try-catch which wraps all uncaught exceptions in WasmEngineException. Not sure if that is a clean solution though, and it might hide actual bugs in Endive.

Side note: Endive's ExecutionListener was pretty useful for the fuzzing setup as a way to limit the instruction count. Otherwise the fuzzer would have got stuck when executing infinite loops in the Wasm code. The interrupt approach might have worked as well, but I guess it has more overhead due to needing to run in a separate thread; and choosing a good timeout which ensures that a large amount of instructions are actually run but without slowing fuzzer progress down too much while waiting for the timeout might be difficult as well.

Jazzer setup (click to expand)

import com.code_intelligence.jazzer.junit.FuzzTest;
import com.code_intelligence.jazzer.mutation.annotation.NotNull;
import com.code_intelligence.jazzer.mutation.annotation.ValuePool;
import org.junit.jupiter.api.Timeout;
import run.endive.runtime.*;
import run.endive.wasm.Parser;
import run.endive.wasm.WasmEngineException;
import run.endive.wasm.WasmModule;
import run.endive.wasm.types.Export;
import run.endive.wasm.types.ExternalType;
import run.endive.wasm.types.FunctionType;
import run.endive.wasm.types.Instruction;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Map;
import java.util.concurrent.TimeUnit;

public class InterpreterTest {
    // Corpus from https://github.com/bytecodealliance/endive/tree/75057984f851fd2b13fb04db42f66ab5600585ae/wasm-corpus/src/main/resources/compiled
    private static final String CORPUS_PATH = "src/test/resources/endive-wasm-corpus/*.wasm";

    private static class InstructionLimitReached extends RuntimeException {
    }

    private static final MethodHandle EXPORTS_FIELD;
    static {
        try {
            Field f = Instance.class.getDeclaredField("exports");
            f.setAccessible(true);
            EXPORTS_FIELD = MethodHandles.lookup().unreflectGetter(f);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }
    private static Export getAnExport(Instance instance) {
        try {
            Map<String, Export> exports = (Map<String, Export>) EXPORTS_FIELD.invoke(instance);
            return exports.values().stream().filter(e -> e.exportType() == ExternalType.FUNCTION).findAny().orElse(null);
        } catch (Throwable e) {
            throw new RuntimeException(e);
        }
    }

    private static long[] adjustArgs(Instance instance, Export export, long[] args) {
        FunctionType type = instance.type(instance.functionType(export.index()));
        int expectedArgsCount = type.params().size();
        // Shorten or extend as necessary
        return Arrays.copyOf(args, expectedArgsCount);
    }

    @Timeout(value = 10, unit = TimeUnit.SECONDS)
    @FuzzTest
    void runWithListener(byte @ValuePool(files = CORPUS_PATH) @NotNull [] bytes, long @NotNull [] args) throws Throwable {
        WasmModule module;
        try {
            module = Parser.parse(bytes);
        } catch (Exception | AssertionError | OutOfMemoryError e) {
            return;
        }

        Instance instance;
        try {
            instance = Instance.builder(module)
                // Run with execution listener
                // https://endive.run/docs/advanced/cpu-limits/#unsafe-execution-listener
                .withUnsafeExecutionListener(new ExecutionListener() {
                    int remainingInstructions = 1_000;

                    @Override
                    public void onExecution(Instruction instruction, MStack stack) {
                        if (remainingInstructions-- < 0) {
                            throw new InstructionLimitReached();
                        }
                    }
                })
                .build();
        } catch (WasmEngineException e) {
            return;
        }

        Export export = getAnExport(instance);
        if (export == null) {
            return;
        }
        args = adjustArgs(instance, export, args);
        ExportFunction exportFunction;
        try {
            exportFunction = instance.export(export.name());
        } catch (WasmEngineException e) {
            return;
        }

        try {
            exportFunction.apply(args);
        } catch (WasmEngineException | WasmException e) {
            return;
        } catch (InstructionLimitReached e) {
            return;
        }
    }
}

Hopefully this is useful for you! Maybe it also helps detecting incorrect Endive behavior where it should reject input but doesn't, or where it fails to execute valid Wasm code.

Though I can understand that improving exception handling might be difficult without making the code overly verbose or decreasing performance.

[andreaTP]

Thanks for the fuzzing work and the detailed report!

I think this is mostly cosmetic, regardless of the exception type, the parser aborts on malformed input, which is the correct behavior.
This is consistent with how most runtimes handle it afaik.

That said, the AssertionError on line 785 should be a proper exception for example, if you spot easy wins like that, PRs are welcome!