cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Always copy dynamically-sized memory arrays during CompilerUtils::abiDecode.

Browse Source
This commit is contained in:
Daniel Kirchner 2019-08-05 14:07:55 +02:00
parent 967ee944a5
commit efb34bdf22
3 changed files with 45 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changelog.md
View File

@ -18,6 +18,7 @@ Compiler Features:
Bugfixes: 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 internal error when inlining functions that contain tuple expressions.
* SMTChecker: Fix pointer knowledge erasing in loops. * SMTChecker: Fix pointer knowledge erasing in loops.
* View/Pure Checker: Properly detect state variable access through base class. * View/Pure Checker: Properly detect state variable access through base class.

21
libsolidity/codegen/CompilerUtils.cpp
View File

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

26
test/libsolidity/semanticTests/abiDecodeV1/dynamic_memory_copy.sol Normal file
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