Merge pull request #6771 from ethereum/fixConstructorABIV2

Fix handling of structs of dynamic size as constructor parameters.
This commit is contained in:
chriseth 2019-05-16 20:53:16 +02:00 committed by GitHub
commit 315f66fc93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 123 additions and 27 deletions

View File

@ -5,6 +5,7 @@ Language Features:
Compiler Features:
* Code Generator: Fix handling of structs of dynamic size as constructor parameters.
* Optimizer: Add rule to simplify SHL/SHR combinations.
* SMTChecker: Support inherited state variables.
* SMTChecker: Support tuples and function calls with multiple return values.

View File

@ -1,4 +1,15 @@
[
{
"name": "DynamicConstructorArgumentsClippedABIV2",
"summary": "A contract's constructor that takes structs or arrays that contain dynamically-sized arrays reverts or decodes to invalid data.",
"description": "During construction of a contract, constructor parameters are copied from the code section to memory for decoding. The amount of bytes to copy was calculated incorrectly in case all parameters are statically-sized but contain dynamically-sized arrays as struct members or inner arrays. Such types are only available if ABIEncoderV2 is activated.",
"introduced": "0.4.16",
"fixed": "0.5.9",
"severity": "very low",
"conditions": {
"ABIEncoderV2": true
}
},
{
"name": "UninitializedFunctionPointerInConstructor",
"summary": "Calling uninitialized internal function pointers created in the constructor does not always revert and can cause unexpected behaviour.",

View File

@ -452,6 +452,7 @@
},
"0.4.16": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ExpExponentCleanup",
@ -462,6 +463,7 @@
},
"0.4.17": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ExpExponentCleanup",
@ -473,6 +475,7 @@
},
"0.4.18": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ExpExponentCleanup",
@ -483,6 +486,7 @@
},
"0.4.19": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -510,6 +514,7 @@
},
"0.4.20": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -521,6 +526,7 @@
},
"0.4.21": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -532,6 +538,7 @@
},
"0.4.22": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -543,6 +550,7 @@
},
"0.4.23": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -553,6 +561,7 @@
},
"0.4.24": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x",
@ -563,6 +572,7 @@
},
"0.4.25": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor_0.4.x",
"IncorrectEventSignatureInLibraries_0.4.x",
"ABIEncoderV2PackedStorage_0.4.x"
@ -570,7 +580,9 @@
"released": "2018-09-12"
},
"0.4.26": {
"bugs": [],
"bugs": [
"DynamicConstructorArgumentsClippedABIV2"
],
"released": "2019-04-29"
},
"0.4.3": {
@ -677,6 +689,7 @@
},
"0.5.0": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage"
@ -685,6 +698,7 @@
},
"0.5.1": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage"
@ -693,6 +707,7 @@
},
"0.5.2": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage"
@ -701,6 +716,7 @@
},
"0.5.3": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage"
@ -709,6 +725,7 @@
},
"0.5.4": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage"
@ -717,6 +734,7 @@
},
"0.5.5": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage",
@ -727,6 +745,7 @@
},
"0.5.6": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries",
"ABIEncoderV2PackedStorage",
@ -736,13 +755,16 @@
},
"0.5.7": {
"bugs": [
"DynamicConstructorArgumentsClippedABIV2",
"UninitializedFunctionPointerInConstructor",
"IncorrectEventSignatureInLibraries"
],
"released": "2019-03-26"
},
"0.5.8": {
"bugs": [],
"bugs": [
"DynamicConstructorArgumentsClippedABIV2"
],
"released": "2019-04-30"
}
}

View File

@ -228,34 +228,21 @@ void ContractCompiler::appendConstructor(FunctionDefinition const& _constructor)
// copy constructor arguments from code to memory and then to stack, they are supplied after the actual program
if (!_constructor.parameters().empty())
{
unsigned argumentSize = 0;
for (ASTPointer<VariableDeclaration> const& var: _constructor.parameters())
if (var->annotation().type->isDynamicallySized())
{
argumentSize = 0;
break;
}
else
argumentSize += var->annotation().type->calldataEncodedSize();
CompilerUtils(m_context).fetchFreeMemoryPointer();
if (argumentSize == 0)
{
// argument size is dynamic, use CODESIZE to determine it
m_context.appendProgramSize(); // program itself
// CODESIZE is program plus manually added arguments
m_context << Instruction::CODESIZE << Instruction::SUB;
}
else
m_context << u256(argumentSize);
// CODESIZE returns the actual size of the code,
// which is the size of the generated code (``programSize``)
// plus the constructor arguments added to the transaction payload.
m_context.appendProgramSize();
m_context << Instruction::CODESIZE << Instruction::SUB;
// stack: <memptr> <argument size>
m_context << Instruction::DUP1;
m_context.appendProgramSize();
m_context << Instruction::DUP4 << Instruction::CODECOPY;
m_context << Instruction::DUP2 << Instruction::ADD;
m_context << Instruction::DUP1;
// stack: <memptr> <argument size>
m_context << Instruction::DUP2 << Instruction::DUP2 << Instruction::ADD;
// stack: <memptr> <argument size> <mem end>
CompilerUtils(m_context).storeFreeMemoryPointer();
// stack: <memptr>
// stack: <memptr> <argument size>
CompilerUtils(m_context).abiDecode(FunctionType(_constructor).parameterTypes(), true);
}
_constructor.accept(*this);

View File

@ -712,6 +712,81 @@ BOOST_AUTO_TEST_CASE(packed_structs)
)
}
BOOST_AUTO_TEST_CASE(struct_in_constructor)
{
string sourceCode = R"(
contract C {
struct S {
string a;
uint8 b;
string c;
}
S public x;
constructor(S memory s) public { x = s; }
}
)";
NEW_ENCODER(
compileAndRun(sourceCode, 0, "C", encodeArgs(0x20, 0x60, 0x03, 0x80, 0x00, 0x00));
ABI_CHECK(callContractFunction("x()"), encodeArgs(0x60, 0x03, 0x80, 0x00, 0x00));
)
}
BOOST_AUTO_TEST_CASE(struct_in_constructor_indirect)
{
string sourceCode = R"(
contract C {
struct S {
string a;
uint8 b;
string c;
}
S public x;
constructor(S memory s) public { x = s; }
}
contract D {
function f() public returns (string memory, uint8, string memory) {
C.S memory s;
s.a = "abc";
s.b = 7;
s.c = "def";
C c = new C(s);
return c.x();
}
}
)";
if (dev::test::Options::get().evmVersion().supportsReturndata())
{
NEW_ENCODER(
compileAndRun(sourceCode, 0, "D");
ABI_CHECK(callContractFunction("f()"), encodeArgs(0x60, 7, 0xa0, 3, "abc", 3, "def"));
)
}
}
BOOST_AUTO_TEST_CASE(struct_in_constructor_data_short)
{
string sourceCode = R"(
contract C {
struct S {
string a;
uint8 b;
string c;
}
S public x;
constructor(S memory s) public { x = s; }
}
)";
NEW_ENCODER(
BOOST_CHECK(
compileAndRunWithoutCheck(sourceCode, 0, "C", encodeArgs(0x20, 0x60, 0x03, 0x80, 0x00)).empty()
);
)
}
BOOST_AUTO_TEST_SUITE_END()
}

View File

@ -176,8 +176,8 @@ BOOST_AUTO_TEST_CASE(store_keccak256)
char const* sourceCode = R"(
contract test {
bytes32 public shaValue;
constructor(uint a) public {
shaValue = keccak256(abi.encodePacked(a));
constructor() public {
shaValue = keccak256(abi.encodePacked(this));
}
}
)";

View File

@ -9771,7 +9771,7 @@ BOOST_AUTO_TEST_CASE(calldata_offset)
}
}
)";
compileAndRun(sourceCode, 0, "CB", encodeArgs(u256(0x20)));
compileAndRun(sourceCode, 0, "CB", encodeArgs(u256(0x20), u256(0x00)));
ABI_CHECK(callContractFunction("last()", encodeArgs()), encodeDyn(string("nd")));
}