CVE-2023-31146 (Python, published 2023-05-11T21:15Z)
Description: value: Vyper is a Pythonic smart contract language for the Ethereum virtual machine. Prior to version 0.3.8, during codegen, the length word of a dynarray is written before the data, which can result in out-of-bounds array access in the case where the dynarray is on both the lhs and rhs of an assignment. The issue can cause data corruption across call frames. The expected behavior is to revert due to out-of-bounds array access. Version 0.3.8 contains a patch for this issue.
Severity: nan (CVSS v3 base score: 9.1)
Weakness: CWE-787 \u2014 Out-of-bounds Write
The product writes data past the end, or before the beginning, of the intended buffer.
Repository: https://github.com/vyperlang/vyper
Files changed:
tests/parser/types/test_dynamic_array.py
vyper/codegen/core.py
Commit message: Merge pull request from GHSA-3p37-3636-q8wv

in dynarray_make_setter, the length is copied before the data. when the
dst and src arrays do not overlap, this is not a problem. however, when
the dst and src are the same dynarray, this can lead to a
store-before-load, leading any array bounds checks on the right hand
side to function incorrectly. here is an example:

```vyper
@external
def should_revert() -> DynArray[uint256,3]:
    a: DynArray[uint256, 3] = [1, 2, 3]
    a = empty(DynArray[uint256, 3])
    a = [self.a[0], self.a[1], self.a[2]]
    return a  # if bug: returns [1,2,3]
```

this commit moves the length store to after the data copy in
dynarray_make_setter. for hygiene, it also moves the length store to
after the data copy in several other routines. I left pop_dyn_array()
unchanged, because moving the routine does not actually perform any data
copy, it just writes the new length (and optionally returns a pointer to
the popped item).

Vulnerable code:
            dst = bytes_data_ptr(dst)
            src = bytes_data_ptr(src)

            ret.append(copy_bytes(dst, src, len_, max_bytes))
        # write the length word
        store_length = STORE(dst, len(src.args))
        ann = None
        if src.annotation is not None:
            ann = f"len({src.annotation})"
        store_length = IRnode.from_list(store_length, annotation=ann)
        ret.append(store_length)

            ret.append(STORE(dst, count))

            ret.append(STORE(darray_node, ["add", len_, 1]))

Fixed code:


dynarray_length_no_clobber_cases = [
    # GHSA-3p37-3636-q8wv cases
    """
a: DynArray[uint256,3]

@external
def should_revert() -> DynArray[uint256,3]:
    self.a = [1,2,3]
    self.a = empty(DynArray[uint256,3])
    self.a = [self.a[0], self.a[1], self.a[2]]

    return self.a  # if bug: returns [1,2,3]
    """,
    """
@external
def should_revert() -> DynArray[uint256,3]:
    self.a()
    return self.b() # if bug: returns [1,2,3]

@internal
def a():
    a: uint256 = 0
    b: uint256 = 1
    c: uint256 = 2
    d: uint256 = 3

@internal
def b() -> DynArray[uint256,3]:
    a: DynArray[uint256,3] = empty(DynArray[uint256,3])
    a = [a[0],a[1],a[2]]
    return a
    """,
    """
a: DynArray[uint256,4]

@external
def should_revert() -> DynArray[uint256,4]:
    self.a = [1,2,3]
    self.a = empty(DynArray[uint256,4])
    self.a = [4, self.a[0]]

    return self.a  # if bug: return [4, 4]
    """,
    """
@external
def should_revert() -> DynArray[uint256,4]:
    a: DynArray[uint256, 4] = [1,2,3]
    a = []

    a = [a.pop()]  # if bug: return [1]

    return a
    """,
    """
@external
def should_revert():
    c: DynArray[uint256, 1] = []
    c.append(c[0])
    """,
    """
@external
def should_revert():
    c: DynArray[uint256, 1] = [1]
    c[0] = c.pop()
    """,
    """
@external
def should_revert():
    c: DynArray[DynArray[uint256, 1], 2] = [[]]
    c[0] = c.pop()
    """,
    """
a: DynArray[String[65],2]

@external
def should_revert() -> DynArray[String[65], 2]:
    self.a = ["hello", "world"]
    self.a = []
    self.a = [self.a[0], self.a[1]]

    return self.a  # if bug: return ["hello", "world"]
    """,
]


@pytest.mark.parametrize("code", dynarray_length_no_clobber_cases)
def test_dynarray_length_no_clobber(get_contract, assert_tx_failed, code):
    # check that length is not clobbered before dynarray data copy happens
    c = get_contract(code)
    assert_tx_failed(lambda: c.should_revert())

            dst_ = bytes_data_ptr(dst)
            src_ = bytes_data_ptr(src)

            ret.append(copy_bytes(dst_, src_, len_, max_bytes))

    # copy contents of src dynarray to dst.
    # note that in case src and dst refer to the same dynarray,
    # in order for get_element_ptr oob checks on the src dynarray
    # to work, we need to wait until after the data is copied
    # before we clobber the length word.

        # copy each item

        # write the length word after data is copied
        store_length = STORE(dst, n_items)
        ann = None
        if src.annotation is not None:
            ann = f"len({src.annotation})"
        store_length = IRnode.from_list(store_length, annotation=ann)

        ret.append(store_length)

            # write the length word after data is copied
            ret.append(STORE(dst, count))


            # store new length
            ret.append(STORE(darray_node, ["add", len_, 1]))
            # store new length