diff --git a/advisories/github-reviewed/2024/02/GHSA-9p8r-4xp4-gw5w/GHSA-9p8r-4xp4-gw5w.json b/advisories/github-reviewed/2024/02/GHSA-9p8r-4xp4-gw5w/GHSA-9p8r-4xp4-gw5w.json index e36226671fa..53238efc4d8 100644 --- a/advisories/github-reviewed/2024/02/GHSA-9p8r-4xp4-gw5w/GHSA-9p8r-4xp4-gw5w.json +++ b/advisories/github-reviewed/2024/02/GHSA-9p8r-4xp4-gw5w/GHSA-9p8r-4xp4-gw5w.json @@ -1,13 +1,13 @@ { "schema_version": "1.4.0", "id": "GHSA-9p8r-4xp4-gw5w", - "modified": "2024-02-26T22:06:28Z", + "modified": "2024-06-18T15:16:24Z", "published": "2024-02-26T20:11:07Z", "aliases": [ "CVE-2024-26149" ], "summary": "Vyper's `_abi_decode` vulnerable to Memory Overflow", - "details": "## Summary\n\nIf an excessively large value is specified as the starting index for an array in `_abi_decode`, it can cause the read position to overflow. This results in the decoding of values outside the intended array bounds, potentially leading to bugs in contracts that use arrays within `_abi_decode`. The advisory has been assigned low severity, because it is only observable if there is a memory write between two invocations of `abi_decode` on the same input.\n\n## Proof of Concept\n\n```vyper\nevent Pwn:\n pass\n\n@external\ndef f(x: Bytes[32 * 3]):\n a: Bytes[32] = b\"foo\"\n y: Bytes[32 * 3] = x\n\n decoded_y1: Bytes[32] = _abi_decode(y, Bytes[32])\n a = b\"bar\"\n decoded_y2: Bytes[32] = _abi_decode(y, Bytes[32])\n\n if decoded_y1 != decoded_y2:\n log Pwn()\n```\n\nSending the following calldata results in `Pwn` being emitted. \n\n```\n0xd45754f8\n0000000000000000000000000000000000000000000000000000000000000020\n0000000000000000000000000000000000000000000000000000000000000060\nffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa0\n```", + "details": "## Summary\n\nIf an excessively large value is specified as the starting index for an array in `_abi_decode`, it can cause the read position to overflow. This results in the decoding of values outside the intended array bounds, potentially leading to bugs in contracts that use arrays within `_abi_decode`. The advisory has been assigned low severity, because it is only observable if there is a memory write between two invocations of `abi_decode` on the same input.\n\n## Proof of Concept\n\n```vyper\nevent Pwn:\n pass\n\n@external\ndef f(x: Bytes[32 * 3]):\n a: Bytes[32] = b\"foo\"\n y: Bytes[32 * 3] = x\n\n decoded_y1: Bytes[32] = _abi_decode(y, Bytes[32])\n a = b\"bar\"\n decoded_y2: Bytes[32] = _abi_decode(y, Bytes[32])\n\n if decoded_y1 != decoded_y2:\n log Pwn()\n```\n\nSending the following calldata results in `Pwn` being emitted. \n\n```\n0xd45754f8\n0000000000000000000000000000000000000000000000000000000000000020\n0000000000000000000000000000000000000000000000000000000000000060\nffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa0\n```\n\n### Patches\nPatched in https://github.com/vyperlang/vyper/pull/3925, https://github.com/vyperlang/vyper/pull/4091, https://github.com/vyperlang/vyper/pull/4144, https://github.com/vyperlang/vyper/pull/4060.", "severity": [ { "type": "CVSS_V3", @@ -28,7 +28,7 @@ "introduced": "0" }, { - "last_affected": "0.3.10" + "fixed": "0.4.0" } ] } diff --git a/advisories/github-reviewed/2024/02/GHSA-gp3w-2v2m-p686/GHSA-gp3w-2v2m-p686.json b/advisories/github-reviewed/2024/02/GHSA-gp3w-2v2m-p686/GHSA-gp3w-2v2m-p686.json index d257bcb5384..7011107b5ec 100644 --- a/advisories/github-reviewed/2024/02/GHSA-gp3w-2v2m-p686/GHSA-gp3w-2v2m-p686.json +++ b/advisories/github-reviewed/2024/02/GHSA-gp3w-2v2m-p686/GHSA-gp3w-2v2m-p686.json @@ -1,13 +1,13 @@ { "schema_version": "1.4.0", "id": "GHSA-gp3w-2v2m-p686", - "modified": "2024-02-02T18:44:51Z", + "modified": "2024-06-18T15:17:07Z", "published": "2024-02-02T18:10:10Z", "aliases": [ "CVE-2024-24560" ], "summary": "Vyper's external calls can overflow return data to return input buffer", - "details": "## Summary\n\nWhen calls to external contracts are made, we write the input buffer starting at byte 28, and allocate the return buffer to start at byte 0 (overlapping with the input buffer). When checking `RETURNDATASIZE` for dynamic types, the size is compared only to the minimum allowed size for that type, and not to the returned value's `length`. As a result, malformed return data can cause the contract to mistake data from the input buffer for returndata.\n\nThis advisory is given a severity of \"Low\" because when the called contract returns invalid ABIv2 encoded data, the calling contract can read different invalid data (from the dirty buffer) than the called contract returned.\n\n## Details\n\nWhen arguments are packed for an external call, we create a buffer of size `max(args, return_data) + 32`. The input buffer is placed in this buffer (starting at byte 28), and the return buffer is allocated to start at byte 0. The assumption is that we can reuse the memory becase we will not be able to read past `RETURNDATASIZE`.\n\n```python\nif fn_type.return_type is not None:\n return_abi_t = calculate_type_for_external_return(fn_type.return_type).abi_type\n\n # we use the same buffer for args and returndata,\n # so allocate enough space here for the returndata too.\n buflen = max(args_abi_t.size_bound(), return_abi_t.size_bound())\nelse:\n buflen = args_abi_t.size_bound()\n\nbuflen += 32 # padding for the method id\n```\n\nWhen data is returned, we unpack the return data by starting at byte 0. We check that `RETURNDATASIZE` is greater than the minimum allowed for the returned type:\n```python\nif not call_kwargs.skip_contract_check:\n assertion = IRnode.from_list(\n [\"assert\", [\"ge\", \"returndatasize\", min_return_size]],\n error_msg=\"returndatasize too small\",\n )\n unpacker.append(assertion)\n```\n\nThis check ensures that any dynamic types returned will have a size of at least 64. However, it does not verify that `RETURNDATASIZE` is as large as the `length` word of the dynamic type. \n\nAs a result, if a contract expects a dynamic type to be returned, and the part of the return data that is read as `length` includes a size that is larger than the actual `RETURNDATASIZE`, the return data read from the buffer will overrun the actual return data size and read from the input buffer.\n\n## Proof of Concept\n\nThis contract calls an external contract with two arguments. As the call is made, the buffer includes:\n- byte 28: method_id\n- byte 32: first argument (0)\n- byte 64: second argument (hash)\n\nThe return data buffer begins at byte 0, and will return the returned bytestring, up to a maximum length of 96 bytes.\n\n```python\ninterface Zero:\n def sneaky(a: uint256, b: bytes32) -> Bytes[96]: view\n\n@external\ndef test_sneaky(z: address) -> Bytes[96]:\n return Zero(z).sneaky(0, keccak256(\"oops\"))\n```\nOn the other side, imagine a simple contract that does not, in fact, return a bytestring, but instead returns two uint256s. I've implemented it in Solidity for ease of use with Foundry:\n```solidity\nfunction sneaky(uint a, bytes32 b) external pure returns (uint, uint) {\n return (32, 32);\n}\n```\n\nThe return data will be parsed as a bytestring. The first 32 will point us to byte 32 to read the length. The second 32 will be perceived as the length. It will then read the next 32 bytes from the return data buffer, even though those weren't a part of the return data.\n\nSince these bytes will come from byte 64, we can see above that the hash was placed there in the input buffer.\n\nIf we run the following Foundry test, we can see that this does in fact happen:\n```solidity\nfunction test__sneakyZeroReturn() public {\n ZeroReturn z = new ZeroReturn();\n c = SuperContract(deployer.deploy(\"src/loose/\", \"ret_overflow\", \"\"));\n console.logBytes(c.test_sneaky(address(z)));\n}\n```\n\n```md\nLogs:\n 0xd54c03ccbc84dd6002c98c6df5a828e42272fc54b512ca20694392ca89c4d2c6\n```\n\n## Impact\n\nMalicious or mistaken contracts returning the malformed data can result in overrunning the returned data and reading return data from the input buffer.", + "details": "## Summary\n\nWhen calls to external contracts are made, we write the input buffer starting at byte 28, and allocate the return buffer to start at byte 0 (overlapping with the input buffer). When checking `RETURNDATASIZE` for dynamic types, the size is compared only to the minimum allowed size for that type, and not to the returned value's `length`. As a result, malformed return data can cause the contract to mistake data from the input buffer for returndata.\n\nThis advisory is given a severity of \"Low\" because when the called contract returns invalid ABIv2 encoded data, the calling contract can read different invalid data (from the dirty buffer) than the called contract returned.\n\n## Details\n\nWhen arguments are packed for an external call, we create a buffer of size `max(args, return_data) + 32`. The input buffer is placed in this buffer (starting at byte 28), and the return buffer is allocated to start at byte 0. The assumption is that we can reuse the memory becase we will not be able to read past `RETURNDATASIZE`.\n\n```python\nif fn_type.return_type is not None:\n return_abi_t = calculate_type_for_external_return(fn_type.return_type).abi_type\n\n # we use the same buffer for args and returndata,\n # so allocate enough space here for the returndata too.\n buflen = max(args_abi_t.size_bound(), return_abi_t.size_bound())\nelse:\n buflen = args_abi_t.size_bound()\n\nbuflen += 32 # padding for the method id\n```\n\nWhen data is returned, we unpack the return data by starting at byte 0. We check that `RETURNDATASIZE` is greater than the minimum allowed for the returned type:\n```python\nif not call_kwargs.skip_contract_check:\n assertion = IRnode.from_list(\n [\"assert\", [\"ge\", \"returndatasize\", min_return_size]],\n error_msg=\"returndatasize too small\",\n )\n unpacker.append(assertion)\n```\n\nThis check ensures that any dynamic types returned will have a size of at least 64. However, it does not verify that `RETURNDATASIZE` is as large as the `length` word of the dynamic type. \n\nAs a result, if a contract expects a dynamic type to be returned, and the part of the return data that is read as `length` includes a size that is larger than the actual `RETURNDATASIZE`, the return data read from the buffer will overrun the actual return data size and read from the input buffer.\n\n## Proof of Concept\n\nThis contract calls an external contract with two arguments. As the call is made, the buffer includes:\n- byte 28: method_id\n- byte 32: first argument (0)\n- byte 64: second argument (hash)\n\nThe return data buffer begins at byte 0, and will return the returned bytestring, up to a maximum length of 96 bytes.\n\n```python\ninterface Zero:\n def sneaky(a: uint256, b: bytes32) -> Bytes[96]: view\n\n@external\ndef test_sneaky(z: address) -> Bytes[96]:\n return Zero(z).sneaky(0, keccak256(\"oops\"))\n```\nOn the other side, imagine a simple contract that does not, in fact, return a bytestring, but instead returns two uint256s. I've implemented it in Solidity for ease of use with Foundry:\n```solidity\nfunction sneaky(uint a, bytes32 b) external pure returns (uint, uint) {\n return (32, 32);\n}\n```\n\nThe return data will be parsed as a bytestring. The first 32 will point us to byte 32 to read the length. The second 32 will be perceived as the length. It will then read the next 32 bytes from the return data buffer, even though those weren't a part of the return data.\n\nSince these bytes will come from byte 64, we can see above that the hash was placed there in the input buffer.\n\nIf we run the following Foundry test, we can see that this does in fact happen:\n```solidity\nfunction test__sneakyZeroReturn() public {\n ZeroReturn z = new ZeroReturn();\n c = SuperContract(deployer.deploy(\"src/loose/\", \"ret_overflow\", \"\"));\n console.logBytes(c.test_sneaky(address(z)));\n}\n```\n\n```md\nLogs:\n 0xd54c03ccbc84dd6002c98c6df5a828e42272fc54b512ca20694392ca89c4d2c6\n```\n\n### Patches\nPatched in https://github.com/vyperlang/vyper/pull/3925, https://github.com/vyperlang/vyper/pull/4091, https://github.com/vyperlang/vyper/pull/4144, https://github.com/vyperlang/vyper/pull/4060.\n\n## Impact\n\nMalicious or mistaken contracts returning the malformed data can result in overrunning the returned data and reading return data from the input buffer.", "severity": [ { "type": "CVSS_V3", @@ -28,7 +28,7 @@ "introduced": "0" }, { - "last_affected": "0.3.10" + "fixed": "0.4.0" } ] }