Merge pull request #4224 from ethereum/revert_wrong_calldata

[BREAKING] Revert if calldata has wrong size
This commit is contained in:
chriseth 2018-07-02 12:55:22 +02:00 committed by GitHub
commit 85b9d3927a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 39 additions and 58 deletions

View File

@ -8,6 +8,7 @@ How to update your code:
Breaking Changes: Breaking Changes:
* ABI Encoder: Properly pad data from calldata (``msg.data`` and external function parameters). Use ``abi.encodePacked`` for unpadded encoding. * ABI Encoder: Properly pad data from calldata (``msg.data`` and external function parameters). Use ``abi.encodePacked`` for unpadded encoding.
* Code Generator: Signed right shift uses proper arithmetic shift, i.e. rounding towards negative infinity. Warning: this may silently change the semantics of existing code! * Code Generator: Signed right shift uses proper arithmetic shift, i.e. rounding towards negative infinity. Warning: this may silently change the semantics of existing code!
* Code Generator: Revert at runtime if calldata is too short or points out of bounds. This is done inside the ``ABI decoder`` and therefore also applies to ``abi.decode()``.
* Commandline interface: Remove obsolete ``--formal`` option. * Commandline interface: Remove obsolete ``--formal`` option.
* Commandline interface: Rename the ``--julia`` option to ``--yul``. * Commandline interface: Rename the ``--julia`` option to ``--yul``.
* Commandline interface: Require ``-`` if standard input is used as source. * Commandline interface: Require ``-`` if standard input is used as source.

View File

@ -181,7 +181,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
} }
} }
void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMemory, bool _revertOnOutOfBounds) void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMemory)
{ {
/// Stack: <source_offset> <length> /// Stack: <source_offset> <length>
if (m_context.experimentalFeatureActive(ExperimentalFeature::ABIEncoderV2)) if (m_context.experimentalFeatureActive(ExperimentalFeature::ABIEncoderV2))
@ -194,14 +194,10 @@ void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMem
} }
//@todo this does not yet support nested dynamic arrays //@todo this does not yet support nested dynamic arrays
size_t encodedSize = 0;
if (_revertOnOutOfBounds) for (auto const& t: _typeParameters)
{ encodedSize += t->decodingType()->calldataEncodedSize(true);
size_t encodedSize = 0; m_context.appendInlineAssembly("{ if lt(len, " + to_string(encodedSize) + ") { revert(0, 0) } }", {"len"});
for (auto const& t: _typeParameters)
encodedSize += t->decodingType()->calldataEncodedSize(true);
m_context.appendInlineAssembly("{ if lt(len, " + to_string(encodedSize) + ") { revert(0, 0) } }", {"len"});
}
m_context << Instruction::DUP2 << Instruction::ADD; m_context << Instruction::DUP2 << Instruction::ADD;
m_context << Instruction::SWAP1; m_context << Instruction::SWAP1;
@ -231,26 +227,21 @@ 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;
if (_revertOnOutOfBounds) // Check that the data pointer is valid and that length times
{ // item size is still inside the range.
// Check that the data pointer is valid and that length times Whiskers templ(R"({
// item size is still inside the range. if gt(ptr, 0x100000000) { revert(0, 0) }
Whiskers templ(R"({ ptr := add(ptr, base_offset)
if gt(ptr, 0x100000000) { revert(0, 0) } let array_data_start := add(ptr, 0x20)
ptr := add(ptr, base_offset) if gt(array_data_start, input_end) { revert(0, 0) }
let array_data_start := add(ptr, 0x20) let array_length := mload(ptr)
if gt(array_data_start, input_end) { revert(0, 0) } if or(
let array_length := mload(ptr) gt(array_length, 0x100000000),
if or( gt(add(array_data_start, mul(array_length, <item_size>)), input_end)
gt(array_length, 0x100000000), ) { revert(0, 0) }
gt(add(array_data_start, mul(array_length, <item_size>)), input_end) })");
) { revert(0, 0) } templ("item_size", to_string(arrayType.isByteArray() ? 1 : arrayType.baseType()->calldataEncodedSize(true)));
})"); m_context.appendInlineAssembly(templ.render(), {"input_end", "base_offset", "offset", "ptr"});
templ("item_size", to_string(arrayType.isByteArray() ? 1 : arrayType.baseType()->calldataEncodedSize(true)));
m_context.appendInlineAssembly(templ.render(), {"input_end", "base_offset", "offset", "ptr"});
}
else
m_context << Instruction::DUP3 << Instruction::ADD;
// stack: v1 v2 ... v(k-1) input_end base_offset current_offset v(k) // stack: v1 v2 ... v(k-1) input_end base_offset current_offset v(k)
moveIntoStack(3); moveIntoStack(3);
m_context << u256(0x20) << Instruction::ADD; m_context << u256(0x20) << Instruction::ADD;
@ -273,30 +264,25 @@ void CompilerUtils::abiDecode(TypePointers const& _typeParameters, bool _fromMem
loadFromMemoryDynamic(IntegerType(256), !_fromMemory); loadFromMemoryDynamic(IntegerType(256), !_fromMemory);
m_context << Instruction::SWAP1; m_context << Instruction::SWAP1;
// stack: input_end base_offset next_pointer data_offset // stack: input_end base_offset next_pointer data_offset
if (_revertOnOutOfBounds) m_context.appendInlineAssembly("{ if gt(data_offset, 0x100000000) { revert(0, 0) } }", {"data_offset"});
m_context.appendInlineAssembly("{ if gt(data_offset, 0x100000000) { revert(0, 0) } }", {"data_offset"});
m_context << Instruction::DUP3 << Instruction::ADD; m_context << Instruction::DUP3 << Instruction::ADD;
// stack: input_end base_offset next_pointer array_head_ptr // stack: input_end base_offset next_pointer array_head_ptr
if (_revertOnOutOfBounds) m_context.appendInlineAssembly(
m_context.appendInlineAssembly( "{ if gt(add(array_head_ptr, 0x20), input_end) { revert(0, 0) } }",
"{ if gt(add(array_head_ptr, 0x20), input_end) { revert(0, 0) } }", {"input_end", "base_offset", "next_ptr", "array_head_ptr"}
{"input_end", "base_offset", "next_ptr", "array_head_ptr"} );
);
// retrieve length // retrieve length
loadFromMemoryDynamic(IntegerType(256), !_fromMemory, true); loadFromMemoryDynamic(IntegerType(256), !_fromMemory, true);
// stack: input_end base_offset next_pointer array_length data_pointer // stack: input_end base_offset next_pointer array_length data_pointer
m_context << Instruction::SWAP2; m_context << Instruction::SWAP2;
// stack: input_end base_offset data_pointer array_length next_pointer // stack: input_end base_offset data_pointer array_length next_pointer
if (_revertOnOutOfBounds) unsigned itemSize = arrayType.isByteArray() ? 1 : arrayType.baseType()->calldataEncodedSize(true);
{ m_context.appendInlineAssembly(R"({
unsigned itemSize = arrayType.isByteArray() ? 1 : arrayType.baseType()->calldataEncodedSize(true); if or(
m_context.appendInlineAssembly(R"({ gt(array_length, 0x100000000),
if or( gt(add(data_ptr, mul(array_length, )" + to_string(itemSize) + R"()), input_end)
gt(array_length, 0x100000000), ) { revert(0, 0) }
gt(add(data_ptr, mul(array_length, )" + to_string(itemSize) + R"()), input_end) })", {"input_end", "base_offset", "data_ptr", "array_length", "next_ptr"});
) { revert(0, 0) }
})", {"input_end", "base_offset", "data_ptr", "array_length", "next_ptr"});
}
} }
else else
{ {

View File

@ -102,7 +102,7 @@ public:
/// area. Also has a hard cap of 0x100000000 for any given length/offset field. /// area. Also has a hard cap of 0x100000000 for any given length/offset field.
/// Stack pre: <source_offset> <length> /// Stack pre: <source_offset> <length>
/// Stack post: <value0> <value1> ... <valuen> /// Stack post: <value0> <value1> ... <valuen>
void abiDecode(TypePointers const& _typeParameters, bool _fromMemory = false, bool _revertOnOutOfBounds = false); void abiDecode(TypePointers const& _typeParameters, bool _fromMemory = false);
/// Copies values (of types @a _givenTypes) given on the stack to a location in memory given /// Copies values (of types @a _givenTypes) given on the stack to a location in memory given
/// at the stack top, encoding them according to the ABI as the given types @a _targetTypes. /// at the stack top, encoding them according to the ABI as the given types @a _targetTypes.

View File

@ -2049,7 +2049,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
mstore(0x40, newMem) mstore(0x40, newMem)
})", {"start", "size"}); })", {"start", "size"});
utils().abiDecode(returnTypes, true, true); utils().abiDecode(returnTypes, true);
} }
} }

View File

@ -266,7 +266,6 @@ BOOST_AUTO_TEST_CASE(calldata_arrays_too_large)
} }
} }
)"; )";
bool newEncoder = false;
BOTH_ENCODERS( BOTH_ENCODERS(
compileAndRun(sourceCode); compileAndRun(sourceCode);
bytes args = encodeArgs( bytes args = encodeArgs(
@ -275,9 +274,8 @@ BOOST_AUTO_TEST_CASE(calldata_arrays_too_large)
); );
ABI_CHECK( ABI_CHECK(
callContractFunction("f(uint256,uint256[],uint256)", args), callContractFunction("f(uint256,uint256[],uint256)", args),
newEncoder ? encodeArgs() : encodeArgs(7) encodeArgs()
); );
newEncoder = true;
) )
} }
@ -449,13 +447,11 @@ BOOST_AUTO_TEST_CASE(short_input_value_type)
function f(uint a, uint b) public pure returns (uint) { return a; } function f(uint a, uint b) public pure returns (uint) { return a; }
} }
)"; )";
bool newDecoder = false;
BOTH_ENCODERS( BOTH_ENCODERS(
compileAndRun(sourceCode); compileAndRun(sourceCode);
ABI_CHECK(callContractFunction("f(uint256,uint256)", 1, 2), encodeArgs(1)); ABI_CHECK(callContractFunction("f(uint256,uint256)", 1, 2), encodeArgs(1));
ABI_CHECK(callContractFunctionNoEncoding("f(uint256,uint256)", bytes(64, 0)), encodeArgs(0)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256,uint256)", bytes(64, 0)), encodeArgs(0));
ABI_CHECK(callContractFunctionNoEncoding("f(uint256,uint256)", bytes(63, 0)), newDecoder ? encodeArgs() : encodeArgs(0)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256,uint256)", bytes(63, 0)), encodeArgs());
newDecoder = true;
) )
} }
@ -466,15 +462,13 @@ BOOST_AUTO_TEST_CASE(short_input_array)
function f(uint[] a) public pure returns (uint) { return 7; } function f(uint[] a) public pure returns (uint) { return 7; }
} }
)"; )";
bool newDecoder = false;
BOTH_ENCODERS( BOTH_ENCODERS(
compileAndRun(sourceCode); compileAndRun(sourceCode);
ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 0)), encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 0)), encodeArgs(7));
ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1)), newDecoder ? encodeArgs() : encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1)), encodeArgs());
ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1) + bytes(31, 0)), newDecoder ? encodeArgs() : encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1) + bytes(31, 0)), encodeArgs());
ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1) + bytes(32, 0)), encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 1) + bytes(32, 0)), encodeArgs(7));
ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 2, 5, 6)), encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("f(uint256[])", encodeArgs(0x20, 2, 5, 6)), encodeArgs(7));
newDecoder = true;
) )
} }