CVE-2024-22419: Out-of-bounds Write (CWE-787)
value: Vyper is a Pythonic Smart Contract Language for the Ethereum Virtual Machine. The `concat` built-in can write over the bounds of the memory buffer that was allocated for it and thus overwrite existing valid data. The root cause is that the `build_IR` for `concat` doesn't properly adhere to the API of copy functions (for `>=0.3.2` the `copy_bytes` function). A contract search was performed and no vulnerable contracts were found in production. The buffer overflow can result in the change of semantics of the contract. The overflow is length-dependent and thus it might go unnoticed during contract testing. However, certainly not all usages of concat will result in overwritten valid data as we require it to be in an internal function and close to the return statement where other memory allocations don't occur. This issue has been addressed in commit `55e18f6d1` which will be included in future releases. Users are advised to update when possible.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2024-01-18T19:15Z
Repository: https://github.com/vyperlang/vyper
Language: Python
Files changed: tests/functional/builtins/codegen/test_concat.py, vyper/builtins/functions.py
Commit date: 2024-01-18 18:08:03 +0000
Commit message: fix: concat buffer bug (#3738)

the `concat()` builtin did not respect the `copy_bytes()` API, it
allocated a buffer in some cases which did not have enough padding.

patches GHSA-2q8v-3gqq-4f8p

---------

Co-authored-by: cyberthirst <cyberthirst.eth@gmail.com>
Diff: diff --git a/tests/functional/builtins/codegen/test_concat.py b/tests/functional/builtins/codegen/test_concat.py
index 5558138551..7354515989 100644
--- a/tests/functional/builtins/codegen/test_concat.py
+++ b/tests/functional/builtins/codegen/test_concat.py
@@ -53,10 +53,74 @@ def krazykonkat(z: Bytes[10]) -> Bytes[25]:
     assert c.krazykonkat(b"moose") == b"cow horse moose"
 
     print("Passed third concat test")
 
 
+def test_concat_buffer(get_contract):
+    # GHSA-2q8v-3gqq-4f8p
+    code = """
+@internal
+def bar() -> uint256:
+    sss: String[2] = concat("a", "b")
+    return 1
+
+
+@external
+def foo() -> int256:
+    a: int256 = -1
+    b: uint256 = self.bar()
+    return a
+    """
+    c = get_contract(code)
+    assert c.foo() == -1
+
+
+def test_concat_buffer2(get_contract):
+    # GHSA-2q8v-3gqq-4f8p
+    code = """
+i: immutable(int256)
+
+@external
+def __init__():
+    i = -1
+    s: String[2] = concat("a", "b")
+
+@external
+def foo() -> int256:
+    return i
+    """
+    c = get_contract(code)
+    assert c.foo() == -1
+
+
+def test_concat_buffer3(get_contract):
+    # GHSA-2q8v-3gqq-4f8p
+    code = """
+s: String[1]
+s2: String[33]
+s3: String[34]
+
+@external
+def __init__():
+    self.s = "a"
+    self.s2 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # 33*'a'
+
+@internal
+def bar() -> uint256:
+    self.s3 = concat(self.s, self.s2)
+    return 1
+
+@external
+def foo() -> int256:
+    i: int256 = -1
+    b: uint256 = self.bar()
+    return i
+    """
+    c = get_contract(code)
+    assert c.foo() == -1
+
+
 def test_concat_bytes32(get_contract_with_gas_estimation):
     test_concat_bytes32 = """
 @external
 def sandwich(inp: Bytes[100], inp2: bytes32) -> Bytes[164]:
     return concat(inp2, inp, inp2)
diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py
index 4f8101dfbe..8ee6f5fd76 100644
--- a/vyper/builtins/functions.py
+++ b/vyper/builtins/functions.py
@@ -541,17 +541,16 @@ class Concat(BuiltinFunctionT):
         if isinstance(args[0].typ, StringT):
             ret_typ = StringT(dst_maxlen)
         else:
             ret_typ = BytesT(dst_maxlen)
 
+        # respect API of copy_bytes
+        bufsize = dst_maxlen + 32
+        buf = context.new_internal_variable(BytesT(bufsize))
+
         # Node representing the position of the output in memory
-        dst = IRnode.from_list(
-            context.new_internal_variable(ret_typ),
-            typ=ret_typ,
-            location=MEMORY,
-            annotation="concat destination",
-        )
+        dst = IRnode.from_list(buf, typ=ret_typ, location=MEMORY, annotation="concat destination")
 
         ret = ["seq"]
         # stack item representing our current offset in the dst buffer
         ofst = "concat_ofst"
 

Vulnerable code:         dst = IRnode.from_list(
            context.new_internal_variable(ret_typ),
            typ=ret_typ,
            location=MEMORY,
            annotation="concat destination",
        )
Fixed code: def test_concat_buffer(get_contract):
    # GHSA-2q8v-3gqq-4f8p
    code = """
@internal
def bar() -> uint256:
    sss: String[2] = concat("a", "b")
    return 1


@external
def foo() -> int256:
    a: int256 = -1
    b: uint256 = self.bar()
    return a
    """
    c = get_contract(code)
    assert c.foo() == -1


def test_concat_buffer2(get_contract):
    # GHSA-2q8v-3gqq-4f8p
    code = """
i: immutable(int256)

@external
def __init__():
    i = -1
    s: String[2] = concat("a", "b")

@external
def foo() -> int256:
    return i
    """
    c = get_contract(code)
    assert c.foo() == -1


def test_concat_buffer3(get_contract):
    # GHSA-2q8v-3gqq-4f8p
    code = """
s: String[1]
s2: String[33]
s3: String[34]

@external
def __init__():
    self.s = "a"
    self.s2 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # 33*'a'

@internal
def bar() -> uint256:
    self.s3 = concat(self.s, self.s2)
    return 1

@external
def foo() -> int256:
    i: int256 = -1
    b: uint256 = self.bar()
    return i
    """
    c = get_contract(code)
    assert c.foo() == -1


        # respect API of copy_bytes
        bufsize = dst_maxlen + 32
        buf = context.new_internal_variable(BytesT(bufsize))

        dst = IRnode.from_list(buf, typ=ret_typ, location=MEMORY, annotation="concat destination")