Merge pull request #7176 from ethereum/decodeMemoryLocation

Always copy dynamically-sized memory arrays during CompilerUtils::abiDecode
This commit is contained in:
chriseth 2019-08-07 15:42:14 +02:00 committed by GitHub
commit c8f04b88bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 3 deletions

View File

@ -20,6 +20,7 @@ Compiler Features:
Bugfixes:
* ABI decoder: Ensure that decoded arrays always point to distinct memory locations.
* SMTChecker: Fix internal error when inlining functions that contain tuple expressions.
* SMTChecker: Fix pointer knowledge erasing in loops.
* SMTChecker: Fix internal error when using compound bitwise assignment operators inside branches.

View File

@ -282,6 +282,15 @@ void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMem
{
// compute data pointer
m_context << Instruction::DUP1 << Instruction::MLOAD;
// stack: v1 v2 ... v(k-1) input_end base_offset current_offset data_offset
fetchFreeMemoryPointer();
// stack: v1 v2 ... v(k-1) input_end base_offset current_offset data_offset dstmem
moveIntoStack(4);
// stack: v1 v2 ... v(k-1) dstmem input_end base_offset current_offset data_offset
m_context << Instruction::DUP5;
// stack: v1 v2 ... v(k-1) dstmem input_end base_offset current_offset data_offset dstmem
// Check that the data pointer is valid and that length times
// item size is still inside the range.
Whiskers templ(R"({
@ -294,11 +303,17 @@ void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMem
gt(array_length, 0x100000000),
gt(add(array_data_start, mul(array_length, <item_size>)), input_end)
) { revert(0, 0) }
mstore(dst, array_length)
dst := add(dst, 0x20)
})");
templ("item_size", to_string(arrayType.calldataStride()));
m_context.appendInlineAssembly(templ.render(), {"input_end", "base_offset", "offset", "ptr"});
// stack: v1 v2 ... v(k-1) input_end base_offset current_offset v(k)
moveIntoStack(3);
m_context.appendInlineAssembly(templ.render(), {"input_end", "base_offset", "offset", "ptr", "dst"});
// stack: v1 v2 ... v(k-1) dstmem input_end base_offset current_offset data_ptr dstdata
m_context << Instruction::SWAP1;
// stack: v1 v2 ... v(k-1) dstmem input_end base_offset current_offset dstdata data_ptr
ArrayUtils(m_context).copyArrayToMemory(arrayType, true);
// stack: v1 v2 ... v(k-1) dstmem input_end base_offset current_offset mem_end
storeFreeMemoryPointer();
m_context << u256(0x20) << Instruction::ADD;
}
else

View File

@ -0,0 +1,26 @@
contract C {
function test(bytes memory buf) public view returns (bool same, bool inplaceDecoded) {
(uint256[] memory arr1, uint256[] memory arr2) = abi.decode(buf, (uint256[],uint256[]));
assembly {
// Check whether arr1 and arr2 end up at the same memory location.
// This used to be the case, if both tail pointers in buf pointed to the
// same memory region, i.e. this used to be false in the first two, but true
// in the last three calls below. The desired behaviour is to always get distinct
// memory regions, i.e. this should be false.
same := eq(arr1, arr2)
// Check whether (given the particular tail pointer of 0x40 for arr1 in the calls below)
// arr1 points to the part of buf containing the encoding of arr1.
// The position of the encoding of arr1 in buf is at offset 0x20 (length) + 0x40 (tail pointer)
// of buf.
// This used to be the case for all the test calls below, whereas now arr1 is always copied
// from buf to a new memory area. Should always be false.
inplaceDecoded := eq(arr1, add(buf, 0x60))
}
}
}
// ----
// test(bytes): 0x20, 0x80, 0x40, 0x60, 0, 0 -> false, false
// test(bytes): 0x20, 0xC0, 0x40, 0x80, 1, 0x42, 1, 0x42 -> false, false
// test(bytes): 0x20, 0x80, 0x40, 0x40, 1, 0x42 -> false, false
// test(bytes): 0x20, 0x60, 0x40, 0x40, 0 -> false, false
// test(bytes): 0x20, 0x80, 0x40, 0x40, 1, 0x42 -> false, false