From f97826d07147ccab522220e84f5259867cfc8e87 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 17:55:04 +0100 Subject: [PATCH 1/5] fix stackoverflow in calldataload, codecopy, extcodecopy + some tests --- libevm/VM.h | 40 +++---- test/stPreCompiledContractsFiller.json | 35 ++++++ test/vm.cpp | 20 ++++ test/vmEnvironmentalInfoTestFiller.json | 140 ++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 20 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index 5fb46fc68..0e27db02d 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -524,12 +524,12 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CALLDATALOAD: { - if ((unsigned)m_stack.back() + 31 < _ext.data.size()) + if ((unsigned)m_stack.back() + (uint64_t)31 < _ext.data.size()) m_stack.back() = (u256)*(h256 const*)(_ext.data.data() + (unsigned)m_stack.back()); else { h256 r; - for (unsigned i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) + for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) r[j] = i < _ext.data.size() ? _ext.data[i] : 0; m_stack.back() = (u256)r; } @@ -540,15 +540,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CALLDATACOPY: { - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = m_stack.back(); + u256 dataIndex = m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.data.size() ? (u256)_ext.data.size() < cf ? 0 : _ext.data.size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.data.data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < dataIndex ? 0 : _ext.data.size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::CODESIZE: @@ -556,15 +556,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CODECOPY: { - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = (u256)m_stack.back(); + u256 dataIndex = (u256)m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.code.size() ? (u256)_ext.code.size() < cf ? 0 : _ext.code.size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.code.data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < dataIndex ? 0 : _ext.code.size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::EXTCODESIZE: @@ -574,15 +574,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con { Address a = asAddress(m_stack.back()); m_stack.pop_back(); - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = m_stack.back(); + u256 dataIndex = m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < cf ? 0 : _ext.codeAt(a).size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.codeAt(a).data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < dataIndex ? 0 : _ext.codeAt(a).size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::GASPRICE: diff --git a/test/stPreCompiledContractsFiller.json b/test/stPreCompiledContractsFiller.json index 9c65ad37b..62a3a1623 100644 --- a/test/stPreCompiledContractsFiller.json +++ b/test/stPreCompiledContractsFiller.json @@ -33,6 +33,41 @@ } }, + "CallEcrecover0_overlappingInputOutput": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "10000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "095e7baea6a6c7c4c2dfeb977efac326af552d87" : { + "balance" : "20000000", + "nonce" : 0, + "code": "{ (MSTORE 0 0x18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c) (MSTORE 32 28) (MSTORE 64 0x73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f) (MSTORE 96 0xeeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549) [[ 2 ]] (CALL 1000 1 0 0 128 64 32) [[ 0 ]] (MOD (MLOAD 64) (EXP 2 160)) [[ 1 ]] (EQ (ORIGIN) (SLOAD 0)) }", + "storage": {} + }, + "a94f5374fce5edbc8e2a8697c15331677e6ebf0b" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "", + "storage": {} + } + }, + "transaction" : { + "nonce" : "0", + "gasPrice" : "1", + "gasLimit" : "365224", + "to" : "095e7baea6a6c7c4c2dfeb977efac326af552d87", + "value" : "100000", + "secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8", + "data" : "" + } + }, + + "CallEcrecover0_completeReturnValue": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", diff --git a/test/vm.cpp b/test/vm.cpp index d7bc0612a..e674d6de3 100644 --- a/test/vm.cpp +++ b/test/vm.cpp @@ -488,6 +488,26 @@ BOOST_AUTO_TEST_CASE(vmLogTest) dev::test::executeTests("vmLogTest", "/VMTests", dev::test::doVMTests); } +BOOST_AUTO_TEST_CASE(vmPerformanceTest) +{ + for (int i = 1; i < boost::unit_test::framework::master_test_suite().argc; ++i) + { + string arg = boost::unit_test::framework::master_test_suite().argv[i]; + if (arg == "--performance") + dev::test::executeTests("vmPerformanceTest", "/VMTests", dev::test::doVMTests); + } +} + +BOOST_AUTO_TEST_CASE(vmArithPerformanceTest) +{ + for (int i = 1; i < boost::unit_test::framework::master_test_suite().argc; ++i) + { + string arg = boost::unit_test::framework::master_test_suite().argv[i]; + if (arg == "--performance") + dev::test::executeTests("vmArithPerformanceTest", "/VMTests", dev::test::doVMTests); + } +} + BOOST_AUTO_TEST_CASE(vmRandom) { string testPath = getTestPath(); diff --git a/test/vmEnvironmentalInfoTestFiller.json b/test/vmEnvironmentalInfoTestFiller.json index 95e7936aa..abeed9945 100644 --- a/test/vmEnvironmentalInfoTestFiller.json +++ b/test/vmEnvironmentalInfoTestFiller.json @@ -338,6 +338,33 @@ } }, + "calldataloadSizeTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ [[ 0 ]] (CALLDATALOAD 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa)}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x123456789abcdef0000000000000000000000000000000000000000000000000024", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, "calldatasize0": { "env" : { @@ -451,6 +478,62 @@ } }, + "calldatacopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CALLDATACOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 0xff ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + + "calldatacopy_DataIndexTooHigh2": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CALLDATACOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 9 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + "calldatacopy1": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", @@ -535,6 +618,34 @@ } }, + "codecopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CODECOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 8 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + "codecopy0": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", @@ -686,6 +797,35 @@ } }, + "extcodecopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (EXTCODECOPY (ADDRESS) 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 8 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + + "extcodecopy0": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", From 80f69b7383b24c4a9a27110b4f6cd4dcd0e311ea Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 18:17:03 +0100 Subject: [PATCH 2/5] avoid code repetition in vm --- libevm/VM.h | 58 +++++++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index 0e27db02d..d846ff079 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -538,50 +538,46 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con case Instruction::CALLDATASIZE: m_stack.push_back(_ext.data.size()); break; - case Instruction::CALLDATACOPY: - { - unsigned offset = (unsigned)m_stack.back(); - m_stack.pop_back(); - u256 dataIndex = m_stack.back(); - m_stack.pop_back(); - unsigned size = (unsigned)m_stack.back(); - m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < dataIndex ? 0 : _ext.data.size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)dataIndex, el); - memset(m_temp.data() + offset + el, 0, size - el); - break; - } case Instruction::CODESIZE: m_stack.push_back(_ext.code.size()); break; - case Instruction::CODECOPY: - { - unsigned offset = (unsigned)m_stack.back(); - m_stack.pop_back(); - u256 dataIndex = (u256)m_stack.back(); - m_stack.pop_back(); - unsigned size = (unsigned)m_stack.back(); - m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < dataIndex ? 0 : _ext.code.size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)dataIndex, el); - memset(m_temp.data() + offset + el, 0, size - el); - break; - } case Instruction::EXTCODESIZE: m_stack.back() = _ext.codeAt(asAddress(m_stack.back())).size(); break; + case Instruction::CALLDATACOPY: + case Instruction::CODECOPY: case Instruction::EXTCODECOPY: { - Address a = asAddress(m_stack.back()); - m_stack.pop_back(); + Address a; + if (inst == Instruction::EXTCODECOPY) + { + a = asAddress(m_stack.back()); + m_stack.pop_back(); + } unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 dataIndex = m_stack.back(); + u256 index = m_stack.back(); m_stack.pop_back(); unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < dataIndex ? 0 : _ext.codeAt(a).size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)dataIndex, el); + unsigned el; + switch(inst) + { + case Instruction::CALLDATACOPY: + el = index + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < index ? 0 : _ext.data.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)index, el); + break; + case Instruction::CODECOPY: + el = index + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < index ? 0 : _ext.code.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)index, el); + break; + case Instruction::EXTCODECOPY: + el = index + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < index ? 0 : _ext.codeAt(a).size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)index, el); + break; + default: + break; + } memset(m_temp.data() + offset + el, 0, size - el); break; } From 2d4d9c7485463d8d65e8a0898a35558d2a871221 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 19:23:48 +0100 Subject: [PATCH 3/5] minor fix --- libevm/VM.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevm/VM.h b/libevm/VM.h index d846ff079..f5feead40 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -529,7 +529,7 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con else { h256 r; - for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) + for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + (uint64_t)32, j = 0; i < e; ++i, ++j) r[j] = i < _ext.data.size() ? _ext.data[i] : 0; m_stack.back() = (u256)r; } From 86794daded9bd911883fcf190eb100f1331f6290 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Fri, 5 Dec 2014 11:34:30 +0100 Subject: [PATCH 4/5] even less code --- libevm/VM.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index f5feead40..be64c0ad1 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -560,24 +560,23 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con m_stack.pop_back(); unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el; + bytes toBeCopied; switch(inst) { case Instruction::CALLDATACOPY: - el = index + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < index ? 0 : _ext.data.size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)index, el); + toBeCopied = _ext.data.toBytes(); break; case Instruction::CODECOPY: - el = index + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < index ? 0 : _ext.code.size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)index, el); + toBeCopied = _ext.code; break; case Instruction::EXTCODECOPY: - el = index + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < index ? 0 : _ext.codeAt(a).size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)index, el); + toBeCopied = _ext.codeAt(a); break; default: break; } + unsigned el = index + (bigint)size > (u256)toBeCopied.size() ? (u256)toBeCopied.size() < index ? 0 : toBeCopied.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, toBeCopied.data() + (unsigned)index, el); memset(m_temp.data() + offset + el, 0, size - el); break; } From 45807bff5ee93a291222d86d0670d6c4e096f153 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Fri, 12 Dec 2014 21:45:10 +0100 Subject: [PATCH 5/5] set first 12 bytes to zero in ecrecover --- libethereum/State.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libethereum/State.cpp b/libethereum/State.cpp index bd5c770cf..30290c315 100644 --- a/libethereum/State.cpp +++ b/libethereum/State.cpp @@ -67,7 +67,7 @@ bytes ecrecoverCode(bytesConstRef _in) secp256k1_start(); if (secp256k1_ecdsa_recover_compact(in.hash.data(), 32, in.r.data(), pubkey, &pubkeylen, 0, (int)(u256)in.v - 27)) ret = dev::sha3(bytesConstRef(&(pubkey[1]), 64)); - + memset(ret.data(), 0, 12); return ret.asBytes(); }