Skip to content

store_probably_utf16_to_latin1_or_utf16 shrinks with alignment 1, so a stored latin1+utf16 string can fail to load #733

Description

@imbrem

store_probably_utf16_to_latin1_or_utf16 requests alignment 1 when shrinking to Latin-1 and does not check the returned pointer’s alignment. But load_string_from_range requires 2-byte alignment for all latin1+utf16 strings. An allocator honoring the requested alignment can therefore return an odd pointer: storing succeeds, but loading the result traps.

This is at a25fc0b, in definitions.py:1673 and CanonicalABI.md:2751:

latin1_size = int(len(encoded) / 2)
for i in range(latin1_size):
  cx.opts.memory[ptr + i] = cx.opts.memory[ptr + 2*i]
ptr = cx.reallocate(ptr, src_byte_length, 1, latin1_size)   # alignment 1
trap_if(ptr + latin1_size > len(cx.opts.memory))             # no alignment check
return (ptr, latin1_size)

The other shrinking path, the final reallocate in store_string_to_latin1_or_utf16, passes alignment 2 and does trap_if(ptr != align_to(ptr, 2)). That matches #54 and #61, which made latin1+utf16 always 2-aligned. Wasmtime's FACT adapter for this case (crates/environ/src/fact/trampoline.rs, commented "Corresponds to store_probably_utf16_to_latin1_or_utf16") also passes alignment 2 to the downsizing realloc and validates the result.

Reproduction

Both the source and destination use latin1+utf16, and the source string is UTF-16-tagged even though every code point fits in latin1. The realloc below honors the requested alignment but moves the buffer to an odd address on the shrink:

import sys
sys.path.insert(0, sys.argv[1])   # .../design/mvp/canonical-abi
from definitions import *

def realloc(args):
    old_ptr, old_size, align, new_size = args
    if old_size == 0:
        return [2]                  # initial allocation: 2-aligned
    assert align == 1               # the shrink asks for alignment 1...
    mem[1:1+new_size] = mem[old_ptr:old_ptr+new_size]
    return [1]                      # ...so an odd pointer meets that alignment

mem = bytearray(64)
opts = CanonicalOptions()
opts.memory = MemInst(mem, 'i32'); opts.string_encoding = 'latin1+utf16'; opts.realloc = realloc
cx = LiftLowerContext(opts, ComponentInstance(Store()))

def run(f):
    task = Task(FuncType([], []), CanonicalOptions(), cx.inst, lambda: [], lambda _: ())
    out = {}
    def body():
        try: out['v'] = f()
        except BaseException as e: out['exc'] = repr(e)
    Thread(task, body).resume()
    return out

stored = run(lambda: store_string_into_range(cx, ('hello', 'latin1+utf16', 5 | (1 << 31))))
print('store:', stored)
print('load :', run(lambda: load_string_from_range(cx, *stored['v'])))

Output:

store: {'v': (1, 5)}
load : {'exc': 'Trap()'}

Suggested fix

Match the other paths and Wasmtime:

-  ptr = cx.reallocate(ptr, src_byte_length, 1, latin1_size)
+  ptr = cx.reallocate(ptr, src_byte_length, 2, latin1_size)
+  trap_if(ptr != align_to(ptr, 2))
   trap_if(ptr + latin1_size > len(cx.opts.memory))

The same change is needed in CanonicalABI.md.

We found this while proving a canonical ABI string round trip in Lean.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions