I suppose, looking at the code, a bool is always represented as u8,
which is what we want anyway, but since these values are not bools but
placeholder integers that must currently always be 0, having them be
bools is kind of strange.
Replaced the reading/writing of them with VarUint7, which is maybe almost
as invalid, but works.
Resolves issue #145.
And added unit test. You can change the `LittleEndian` to `BigEndian`
in `Module::default()` and see what it would otherwise do when built
on a big-endian platform.
Because when I'm trying to debug my program it's nice to be able
to print out the pieces of it.
Also added Copy and Clone to a few types that could use it, just 'cause.
Because `debug_assert_eq!` is implemented using `if
cfg!(debug_assertions)`, we need to avoid referring to `slow_len` unless
it is actually being included in the build.
This addresses the potential security issue discussed in the last patch
by implementing custom versions of `deserialize` that check for objects
corresponding to each name.
Spec: https://github.com/WebAssembly/design/blob/master/BinaryEncoding.md#name-section
Issue: https://github.com/paritytech/parity-wasm#129
This is a basic implementation of *.wasm name sections, including
serialization and deserialization. As per the discussion with @NikVolf
on #129, we implement a custom `IndexMap` type.
Before merging this, we should consider the following issues:
- [ ] SECURITY: The custom `IndexMap` structure is exposed to a denial
of service attack. This can be easily prevented; see below for discussion.
- [ ] We probably want top-level `mod.module_name_section()`,
`mod.function_name_section()` and `mod.local_name_section()` APIs to
make it easy to access the relevant sections. Only one of each section
is allowed, so this would make semantic sense.
- [ ] We need a real-world *.wasm file with a module and local name sections.
- [ ] It looks like name sections will normally be left unparsed. But we
should probably go ahead and hook them up to the fuzzer manually.
- [ ] We currently require all entries in an `IndexMap` to be in
ascending order, as required by the standard. We could relax this
check if we needed to handle non-compliant files.
Security and `IndexMap`:
It's possible to create a local name section containing three entries,
for the indices `u32::MAX-2`, `u32::MAX-1` and `u32::MAX`, each of
which contains an entry for a local variable with index `u32::MAX`. This
will consume an utterly ridiculous amount of RAM.
There are three possible fixes, all of which are relatively easy:
1. Switch from `IndexMap<T>` to `BTreeMap<u32, T>`.
2. Keep the index `IndexMap<T>` API, but reimplement it using a private
`BTreeMap<u32, T>`. This allows us to replace the implementation
later without breaking the API.
3. Keep `IndexMap<T>`, but refuse to deserialize maps containing too
many gaps. This would require a slightly different `Deserialize` API,
particular to deal with `IndexMap<IndexMap<String>>`.