# #38682 \[SC-Medium] AugAssign evaluation order causing OOB write within the object

**Submitted on Jan 10th 2025 at 01:15:10 UTC by @anatomist for** [**Attackathon | Ethereum Protocol**](https://immunefi.com/audit-competition/ethereum-protocol-attackathon)

* **Report ID:** #38682
* **Report Type:** Smart Contract
* **Report severity:** Medium
* **Target:** <https://github.com/vyperlang/vyper>
* **Impacts:**
  * (Compiler) Incorrect bytecode generation leading to incorrect behavior
  * (Compiler) Unexpected behavior

## Description

## Brief/Intro

AugAssign (e.g. `+=`, `-=`, ...) statements does not consider the side effect caused by rhs, allowing out-of-bounds index write within the array.

## Vulnerability Details

Vyper parse `AugAssign` statements by first cache the target location to avoid double evaluation.\
However, in the case when target is an access to a `DynArray` and the right value modify the array, the cached `target` will evaluate first, following the modifications like `DynArray.pop()`, and eventually the `STORE` operation could store the computed output in an index that falls outside the array’s current length bounds.

```python
def parse_AugAssign(self):
	target = self._get_target(self.stmt.target)
	right = Expr.parse_value_expr(self.stmt.value, self.context)

	if not target.typ._is_prim_word:
		# because of this check, we do not need to check for
		# make_setter references lhs<->rhs as in parse_Assign -
		# single word load/stores are atomic.
		raise TypeCheckFailure("unreachable")

	with target.cache_when_complex("_loc") as (b, target):
		left = IRnode.from_list(LOAD(target), typ=target.typ)
		new_val = Expr.handle_binop(self.stmt.op, left, right, self.context)
		return b.resolve(STORE(target, new_val))
```

This is alike [this](https://github.com/vyperlang/vyper/security/advisories/GHSA-3p37-3636-q8wv) high severity issue where it discusses similar issues for `Assign`. However, it is not appropriate to apply the same mitigations in `parse_Assign` to `parse_AugAssign`, since the evaluation order of `Assign` is `rhs -> lhs`, while evaluation of `AugAssign` should be `dereference lhs -> rhs -> assign to lhs`, so assigning `rhs` to a temporary variable will actually change the evaluation order. We currently don't have a simple patch in mind, aside from reimplementing the entire `AugAssign`. Notably, care must be taken to not double evaluate `lhs` when fixing the bug.

## Impact Details

The direct impact is out-of-bounds index access within the array. But wrong IR code emission could also lead to various consequences.

## References

<https://github.com/vyperlang/vyper/blob/a29b49d422f6979be2b9c6c80aa583a60b1ccb7f/vyper/codegen/stmt.py#L291>

## Proof of Concept

## Proof of Concept

```
@deploy
def __init__():
    a: DynArray[uint256, 2] = [1, 2]
    a[1] += a.pop() # should revert here, but doesn't. rhs doesn't have to be pop, and can be other complex functions
```


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/ethereum-protocol-or-attackathon/38682-sc-medium-augassign-evaluation-order-causing-oob-write-within-the-object.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
