From 32b83328673c8e28a71c2d24171b2e60572f4845 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 7 Apr 2021 16:25:59 +0200 Subject: [PATCH] Refactor array decoding. --- libsolidity/codegen/ABIFunctions.cpp | 51 ++++++++++--------- .../offset_overflow_in_array_decoding.sol | 29 +++++++++++ .../offset_overflow_in_array_decoding_2.sol | 30 +++++++++++ .../offset_overflow_in_array_decoding_3.sol | 23 +++++++++ 4 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding.sol create mode 100644 test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_2.sol create mode 100644 test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_3.sol diff --git a/libsolidity/codegen/ABIFunctions.cpp b/libsolidity/codegen/ABIFunctions.cpp index facdb9cb8..cfbb01ec6 100644 --- a/libsolidity/codegen/ABIFunctions.cpp +++ b/libsolidity/codegen/ABIFunctions.cpp @@ -1182,12 +1182,28 @@ string ABIFunctions::abiDecodingFunctionArrayAvailableLength(ArrayType const& _t function (offset, length, end) -> array { array := ((length)) let dst := array - + + mstore(array, length) + dst := add(array, 0x20) + let src := offset - + + // TODO add check that we can actually load from all + // offset pointers, i.e. as below, but with stride being 0x20. + + if gt(add(src, mul(length, )), end) { + + } + for { let i := 0 } lt(i, length) { i := add(i, 1) } { - let elementPos := + + let innerOffset := (src) + // TODO add overflow check + let elementPos := add(offset, innerOffset) + + let elementPos := src + mstore(dst, (elementPos, end)) dst := add(dst, 0x20) src := add(src, ) @@ -1198,28 +1214,15 @@ string ABIFunctions::abiDecodingFunctionArrayAvailableLength(ArrayType const& _t templ("readableTypeName", _type.toString(true)); templ("allocate", m_utils.allocationFunction()); templ("allocationSize", m_utils.arrayAllocationSizeFunction(_type)); - string calldataStride = toCompactHexWithPrefix(_type.calldataStride()); - templ("stride", calldataStride); - if (_type.isDynamicallySized()) - templ("storeLength", "mstore(array, length) dst := add(array, 0x20)"); - else - templ("storeLength", ""); - if (_type.baseType()->isDynamicallyEncoded()) - { - templ("staticBoundsCheck", ""); - string load = _fromMemory ? "mload" : "calldataload"; - templ("retrieveElementPos", "add(offset, " + load + "(src))"); - } - else - { - templ("staticBoundsCheck", "if gt(add(src, mul(length, " + - calldataStride + - ")), end) { " + - revertReasonIfDebug("ABI decoding: invalid calldata array stride") + - " }" + templ("stride", toCompactHexWithPrefix(_type.calldataStride())); + templ("dynamic", _type.isDynamicallySized()); + templ("load", _fromMemory ? "mload" : "calldataload"); + templ("dynamicBase", _type.baseType()->isDynamicallyEncoded()); + if (!_type.baseType()->isDynamicallyEncoded()) + templ( + "revertInvalidStride", + revertReasonIfDebug("ABI decoding: invalid calldata array stride") ); - templ("retrieveElementPos", "src"); - } templ("decodingFun", abiDecodingFunction(*_type.baseType(), _fromMemory, false)); return templ.render(); }); diff --git a/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding.sol b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding.sol new file mode 100644 index 000000000..a7e335e47 --- /dev/null +++ b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding.sol @@ -0,0 +1,29 @@ +pragma abicoder v2; +contract Test { + struct MemoryUint { + uint field; + } + function test() public pure returns (uint) { + uint[] memory before = new uint[](1); // at offset 0x80 + // Two problems here: The first offset is zero, the second offset is missing. + bytes memory corrupt = abi.encode(uint(32), // offset to "tuple" + uint(0)); // bogus first element + /* + At this point the free pointer is 0x80 + 64 (size of before) + 32 (length field of corrupt) + 64 (two encoded words) + + Now let's put random junk into memory immediately after the bogus first element. Our goal is to overflow the read pointer to point to before. + The value read out at this point will be added to beginning of the encoded tuple, AKA corrupt + 64. We need then to write x where: + x + 0x80 + 64 (before) + 32 (length of corrupt) + 32 (first word of corrupt) = 0x80 (mod 2^256) + that is MAX_UINT - 128 + */ + MemoryUint memory afterCorrupt; + afterCorrupt.field = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80); + before[0] = 123456; + uint[][2] memory decoded = abi.decode(corrupt, (uint[][2])); + return decoded[1][0]; + } +} +// ==== +// compileViaYul: also +// ---- +// test() -> 0x01e240 diff --git a/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_2.sol b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_2.sol new file mode 100644 index 000000000..88ff3510b --- /dev/null +++ b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_2.sol @@ -0,0 +1,30 @@ +pragma abicoder v2; +contract Test { + struct MemoryTuple { + uint field1; + uint field2; + } + function withinArray() public pure returns (uint) { + uint[] memory before = new uint[](1); + bytes memory corrupt = abi.encode(uint(32), + uint(2)); + MemoryTuple memory afterCorrupt; + before[0] = 123456; + /* + As above, but in this case we are adding to: + 0x80 + 64 (before) + 32 (length of corrupt) + 32 (offset) + 32 (field pointer) + giving MAX_UINT - 96 + */ + afterCorrupt.field1 = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60); + afterCorrupt.field2 = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60); + uint[][] memory decoded = abi.decode(corrupt, (uint[][])); + /* + Will return 123456 * 2, AKA before has been copied twice + */ + return decoded[0][0] + decoded[1][0]; + } +} +// ==== +// compileViaYul: also +// ---- +// withinArray() -> 0x03c480 diff --git a/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_3.sol b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_3.sol new file mode 100644 index 000000000..30fc8731e --- /dev/null +++ b/test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_3.sol @@ -0,0 +1,23 @@ +pragma abicoder v2; +contract Test { + struct MemoryUint { + uint field; + } + function test() public pure returns (uint) { + uint[] memory before = new uint[](1); // at offset 0x80 + bytes memory corrupt = abi.encode( + uint(32), + uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80), + uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80) + ); + MemoryUint memory afterCorrupt; + afterCorrupt.field = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80); + before[0] = 123456; + uint[][2] memory decoded = abi.decode(corrupt, (uint[][2])); + return decoded[1][0]; + } +} +// ==== +// compileViaYul: also +// ---- +// test() -> 0x01e240