fix[codegen]: fix _abi_decode buffer overflow

[cyberthirst]

What I did

• fix for: _abi_decode Memory Overflow #3899

How I did it

• validate that the head points within the parent bounds

How to verify it

• added the PoC from GHSA-9p8r-4xp4-gw5w as a test

Commit message

this commit fixes two related bugs in the ABI decoder.

the first is that the ABI decoder does not have a buffer overflow check
for "head" (aka, dynamic offsets) pointers. this can result in a toctou,
where the result of ABI decoding a bytearray can depend on the value
allocated after the bytearray in memory, and a change is observable if
there is a memory write between two ABI decodes of the same buffer:

```vyper
def foo(xs: Bytes[1024]):
    y: Bytes[32] = b"foo"
    x: Bytes[1024] = xs
    # the `head` element of `xs` points outside of the `xs` buffer,
    # to `y`
    z1: Bytes[32] = _abi_decode(y, Bytes[32])
    y = b"bar"
    # `z1` != `x2`
    z2: Bytes[32] = _abi_decode(y, Bytes[32])
```

the second is that the "head" pointer can point within the allocated
buffer but not within the payload. for instance, we might allocate 1024
bytes as an upper bound for the payload, but the payload could be just
128 bytes at runtime. if the "head" pointer points to 160, then the
decoder might read dirty memory. we ban this behavior to prevent
introspection of dirty memory.

both of these are only considered security vulnerabilities when the
payload is in memory. if the payload is in calldata (or returndata -
although that is not currently applicable since we do not ABI decode
directly from returndata at this time), the payload is user-controlled,
and the worst they can do is force reads of zero bytes. therefore, the
fix here only does the overflow check for decodes from memory.

an alternative implementation strategy was considered, which is to
decode ABI payloads more "strictly" - requiring that each "head" pointer
is equal the the previous "head" plus the length of that item, but this
was scrapped as it would requires a larger change to the decoder.

Description for the changelog

Cute Animal Picture

[cyberthirst]

TODO: we should likely add a check that the element pointed to by head is also within the parent bounds

[cyberthirst]

also, looking at the code, i think there's a possibility for double eval of parent node

[cyberthirst]

yeah, looks like double eval

[with,
    arr_ptr,
    [add,
      [add, [add, 64 <x>, 32], 32],
      [with,
        clamp_arg,
        [mload, [add, [add, [add, 64 <x>, 32], 32], [mul, copy_darray_ix1, 32]]],
        [seq, [assert, [le, clamp_arg, 512]], clamp_arg]]],

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.94%. Comparing base (003d0c6) to head (538cdaa).
Report is 1 commits behind head on master.

❗ Current head 538cdaa differs from pull request most recent head 898a91d

Please upload reports for the commit 898a91d to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3925      +/-   ##
==========================================
- Coverage   91.04%   87.94%   -3.10%     
==========================================
  Files         107      106       -1     
  Lines       15422    15325      -97     
  Branches     3389     3152     -237     
==========================================
- Hits        14041    13478     -563     
- Misses        947     1314     +367     
- Partials      434      533      +99     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[trocher]

another example in case you want more tests there:

@external
def bar() -> (uint256, uint256, uint256):
    return (480, 0, 0)
    

interface A:
    def bar() -> String[32]: nonpayable

@internal
def internal_func():
    # in this case we extcall ourself for the sake of the POC but it could be any other contract
    x:String[32] = extcall A(self).bar()
    print(x) # prints "oooops"
@external
def foo():
    w:String[8] = "oooops"
    self.internal_func()

[charles-cooper]

this is more or less ready for review, @cyberthirst to add more tests

[charles-cooper]

nice work. thanks!