fix[codegen]: zero-length dynarray abi_decode validation

[charles-cooper]

What I did

fix an edge case in abi decode validation

reported by @chen-robert

How I did it

How to verify it

Commit message

fix an edge case in `abi_decode` dynarray validation. when the child
type is dynamic and the runtime length is zero, the check that the
offset pointer is valid (points within the payload) was skipped.

skipping the offset pointer check is valid any time the runtime
length is nonzero, because the pointer is bounded by the checks in
the recursive runtime loop in `_dynarray_make_setter`. however, it is
invalid to skip the check when the runtime length of the dynarray is
zero, because then the recursive loop does not get run.

the impact of this can be seen in the included test cases, particularly
`test_abi_decode_top_level_head_oob`. although as of eb011367cc769d6
it is impossible to convince the decoder to *copy* oob data since the
validation is only skipped when the length is zero, a payload can be
crafted which will revert depending on if some value outside of the
buffer is nonzero (i.e. the runtime behavior can be influenced by some
data outside of the payload).

this commit fixes this issue by _unconditionally_ checking that the
offset pointer is valid. note that the check is now always performed,
even when the runtime length is nonzero and therefore the check is
redundant (because, as stated, the checks within the loop already bound
the offset pointer).

a more efficient implementation is possible, since the check only needs
to be run in the case that the runtime length is 0, which theoretically
can be merged into the same basic block with the 0-case in the `repeat`
loop. however, this commit leaves that to future optimizer work; the
optimization here is it just avoids the multiplication when the child
type is dynamic (because the result of the multiplication is always 0).

this commit also fixes another bug in dynarray recursion; the
calculation in `_abi_payload_size` was not correct when the size of the
child type is larger than 32.

misc:
- add additional tests for abi_decode validation.

Description for the changelog

Cute Animal Picture

[charles-cooper]

can we get more efficient? this check is redundant except in the case where length is 0 (and therefore the loop is skipped)

[charles-cooper]

can we get more efficient? this check is redundant except in the case where length is 0 (and therefore the loop is skipped)

we could, but i'm going to punt it as an optimizer problem (for @harkal 😀 )

[chen-robert]

lgtm

[cyberthirst]

maybe reformulate the comment a bit - the length check was performed even before. what was missing was the in-bounds check ["assert", ["le", item_end, hi]]

[cyberthirst]

the whole _abi_payload_size function is suspicious - why is it assumed for arrays that the elements are primitive?

this test fails (ie doesn't raise)

def test_abi_decode_dynarray_complex(env, tx_failed, get_contract):
    code = """
struct Point:
    x: uint256
    y: uint256

@external
def run(x: Bytes[32 * 8]):
    y: Bytes[32 * 8] = x
    decoded_y1: DynArray[Point, 3] = _abi_decode(y, DynArray[Point, 3])
    """
    c = get_contract(code)

    payload = (
        0x20,
        0x03,
        *_replicate(0x03, 3),
    )

    data = _abi_payload_from_tuple(payload)

    with tx_failed():
        c.run(data)

the payload for the dynarray is *_replicate(0x03, 3) but we're decoding Points, this surely must be an OOB right?