From 1dc28c065d91416caf778770ef57b73b30462b8d Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 7 May 2018 16:23:39 +0200 Subject: Properly pad data from calldata. --- Changelog.md | 1 + libsolidity/codegen/ArrayUtils.cpp | 17 +++++++++++------ test/libsolidity/ABIEncoderTests.cpp | 7 ++----- test/libsolidity/SolidityEndToEndTest.cpp | 8 +++++--- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/Changelog.md b/Changelog.md index 8c895ee0..e3383eae 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.5.0 (unreleased) Breaking Changes: + * 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! * Commandline interface: Require ``-`` if standard input is used as source. * General: ``continue`` in a ``do...while`` loop jumps to the condition (it used to jump to the loop body). Warning: this may silently change the semantics of existing code. diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 14c887c3..2b77db8f 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -303,12 +303,17 @@ void ArrayUtils::copyArrayToMemory(ArrayType const& _sourceType, bool _padToWord m_context << _sourceType.length(); if (baseSize > 1) m_context << u256(baseSize) << Instruction::MUL; - // stack: target source_offset source_len - m_context << Instruction::DUP1 << Instruction::DUP3 << Instruction::DUP5; - // stack: target source_offset source_len source_len source_offset target - m_context << Instruction::CALLDATACOPY; - m_context << Instruction::DUP3 << Instruction::ADD; - m_context << Instruction::SWAP2 << Instruction::POP << Instruction::POP; + + string routine = "calldatacopy(target, source, len)\n"; + if (_padToWordBoundaries) + routine += R"( + // Set padding suffix to zero + mstore(add(target, len), 0) + len := and(add(len, 0x1f), not(0x1f)) + )"; + routine += "target := add(target, len)\n"; + m_context.appendInlineAssembly("{" + routine + "}", {"target", "source", "len"}); + m_context << Instruction::POP << Instruction::POP; } else if (_sourceType.location() == DataLocation::Memory) { diff --git a/test/libsolidity/ABIEncoderTests.cpp b/test/libsolidity/ABIEncoderTests.cpp index 49db9ce1..5f15b28f 100644 --- a/test/libsolidity/ABIEncoderTests.cpp +++ b/test/libsolidity/ABIEncoderTests.cpp @@ -374,15 +374,12 @@ BOOST_AUTO_TEST_CASE(calldata) )"; string s("abcdef"); string t("abcdefgggggggggggggggggggggggggggggggggggggggghhheeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeggg"); - bool newEncoder = false; BOTH_ENCODERS( compileAndRun(sourceCode); callContractFunction("f(bytes)", 0x20, s.size(), s); - // The old encoder did not pad to multiples of 32 bytes - REQUIRE_LOG_DATA(encodeArgs(0x20, s.size()) + (newEncoder ? encodeArgs(s) : asBytes(s))); + REQUIRE_LOG_DATA(encodeArgs(0x20, s.size(), s)); callContractFunction("f(bytes)", 0x20, t.size(), t); - REQUIRE_LOG_DATA(encodeArgs(0x20, t.size()) + (newEncoder ? encodeArgs(t) : asBytes(t))); - newEncoder = true; + REQUIRE_LOG_DATA(encodeArgs(0x20, t.size(), t)); ) } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 2afb4e1c..236d83ef 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -3457,7 +3457,7 @@ BOOST_AUTO_TEST_CASE(event_really_lots_of_data) callContractFunction("deposit()"); BOOST_REQUIRE_EQUAL(m_logs.size(), 1); BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); - BOOST_CHECK_EQUAL(toHex(m_logs[0].data), toHex(encodeArgs(10, 0x60, 15, 4) + FixedHash<4>(dev::keccak256("deposit()")).asBytes())); + BOOST_CHECK_EQUAL(toHex(m_logs[0].data), toHex(encodeArgs(10, 0x60, 15, 4, asString(FixedHash<4>(dev::keccak256("deposit()")).asBytes())))); BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("Deposit(uint256,bytes,uint256)"))); } @@ -3964,7 +3964,8 @@ BOOST_AUTO_TEST_CASE(call_forward_bytes_length) compileAndRun(sourceCode, 0, "sender"); // No additional data, just function selector - ABI_CHECK(callContractFunction("viaCalldata()"), encodeArgs(4)); + ABI_CHECK(callContractFunction("viaCalldata()"), encodeArgs(0x20)); + // Should be this with 0.5.0: encodeArgs(4)); ABI_CHECK(callContractFunction("viaMemory()"), encodeArgs(0x20)); // Should be this with 0.5.0: encodeArgs(4)); ABI_CHECK(callContractFunction("viaStorage()"), encodeArgs(0x20)); @@ -3972,7 +3973,8 @@ BOOST_AUTO_TEST_CASE(call_forward_bytes_length) // Some additional unpadded data bytes unpadded = asBytes(string("abc")); - ABI_CHECK(callContractFunctionNoEncoding("viaCalldata()", unpadded), encodeArgs(7)); + ABI_CHECK(callContractFunctionNoEncoding("viaCalldata()", unpadded), encodeArgs(0x20)); + // Should be this with 0.5.0: encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("viaMemory()", unpadded), encodeArgs(0x20)); // Should be this with 0.5.0: encodeArgs(7)); ABI_CHECK(callContractFunctionNoEncoding("viaStorage()", unpadded), encodeArgs(0x20)); -- cgit