Skip to content

Commit

Permalink
<fix>(precompiled): fix ecRecover core in balance transfer (FISCO-BCO…
Browse files Browse the repository at this point in the history 
…S#4186)
  • Loading branch information
@JimmyShi22
JimmyShi22 authored Jan 22, 2024
1 parent f5748e9 commit 148f72c
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 41 deletions.
23 changes: 1 addition & 22 deletions bcos-executor/src/precompiled/extension/AccountPrecompiled.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,17 +277,7 @@ void AccountPrecompiled::addAccountBalance(const std::string& accountTableName,
getErrorCodeOut(_callParameters->mutableExecResult(), CODE_NO_AUTHORIZED, codec);
return;
}
if (isPrecompiledAddressRange(accountTableName.substr(USER_APPS_PREFIX.length())))
{
PRECOMPILED_LOG(INFO) << BLOCK_NUMBER(blockContext.number())
<< LOG_BADGE("AccountPrecompiled, addAccountBalance")
<< LOG_DESC("account is precompiled address")
<< LOG_KV(
"account", accountTableName.substr(USER_APPS_PREFIX.length()));
BOOST_THROW_EXCEPTION(
PrecompiledError("addBalance failed, toAddress is precompiledAddress!"));
return;
}

// check account exist
auto table = _executive->storage().openTable(accountTableName);
if (!table.has_value()) [[unlikely]]
Expand Down Expand Up @@ -376,17 +366,6 @@ void AccountPrecompiled::subAccountBalance(const std::string& accountTableName,
getErrorCodeOut(_callParameters->mutableExecResult(), CODE_NO_AUTHORIZED, codec);
return;
}
// check account is precompiled address, if true, return
if (isPrecompiledAddressRange(accountTableName.substr(USER_APPS_PREFIX.length())))
{
PRECOMPILED_LOG(INFO) << BLOCK_NUMBER(blockContext.number())
<< LOG_BADGE("AccountPrecompiled, addAccountBalance")
<< LOG_DESC("account is precompiled address")
<< LOG_KV("account", accountTableName.substr(6));
BOOST_THROW_EXCEPTION(
PrecompiledError("addBalance failed, toAddress is precompiledAddress!"));
return;
}

// check account exist
auto table = _executive->storage().openTable(accountTableName);
Expand Down
15 changes: 12 additions & 3 deletions bcos-executor/src/vm/Precompiled.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include "wedpr-crypto/WedprBn128.h"
#include "wedpr-crypto/WedprCrypto.h"
#include <bcos-utilities/Log.h>
#include <algorithm>


using namespace std;
using namespace bcos;
Expand Down Expand Up @@ -470,10 +472,17 @@ bytes blake2FCompression(uint32_t _rounds, bytesConstRef _stateVector, bytesCons
const int RSV_LENGTH = 65;
const int PUBLIC_KEY_LENGTH = 64;
pair<bool, bytes> ecRecover(bytesConstRef _in)
{ // _in is hash(32),v(32),r(32),s(32), return address
{ // _in is hash(32),v(32),r(32),s(32), return address
if (_in.size() <= 128 - 32) // must has hash(32),v(32),r(32),s(32)
{
BCOS_LOG(TRACE) << LOG_BADGE("Precompiled")
<< LOG_DESC("ecRecover: must has hash(32),v(32),r(32),s(32)");
return {true, {}};
}

BCOS_LOG(TRACE) << LOG_BADGE("Precompiled") << LOG_DESC("ecRecover: ") << _in.size();
byte rawRSV[RSV_LENGTH];
memcpy(rawRSV, _in.data() + 64, RSV_LENGTH - 1);
byte rawRSV[RSV_LENGTH] = {0};
memcpy(rawRSV, _in.data() + 64, std::min(_in.size() - 64, (size_t)(RSV_LENGTH - 1)));
rawRSV[RSV_LENGTH - 1] = (byte)((int)_in[63] - 27);
crypto::HashType mHash;
memcpy(mHash.data(), _in.data(), crypto::HashType::SIZE);
Expand Down
89 changes: 89 additions & 0 deletions bcos-executor/test/unittest/libvm/PrecompiledTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,39 @@ BOOST_AUTO_TEST_CASE(ecrecoverSuccessTest)
BOOST_CHECK_EQUAL(address, "0x6da0599583855f1618b380f6782c0c5c25cb96ec");
}

BOOST_AUTO_TEST_CASE(ecrecoverSuccessTest2)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001b" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b" // r
"01932751f4431c3b4c9d6fb1c826d138ee155ea72ac9013d66929f6a265386b4"); // s

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success);
auto address =
*bcos::toHexString(addressBytes.data() + 12, addressBytes.data() + 12 + 20, "0x");
// 0xA5b4792dcAD4fE78D13f6Abd7BA1F302945DE4f7 lower cases
BOOST_CHECK_EQUAL(address, "0xa5b4792dcad4fe78d13f6abd7ba1f302945de4f7");
}

BOOST_AUTO_TEST_CASE(ecrecoverSuccessLargerTest)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001b" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b" // r
"01932751f4431c3b4c9d6fb1c826d138ee155ea72ac9013d66929f6a265386b4" // s
"aabbccddeeff"); // no used extra data

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success);
auto address =
*bcos::toHexString(addressBytes.data() + 12, addressBytes.data() + 12 + 20, "0x");
// 0xA5b4792dcAD4fE78D13f6Abd7BA1F302945DE4f7 lower cases
BOOST_CHECK_EQUAL(address, "0xa5b4792dcad4fe78d13f6abd7ba1f302945de4f7");
}

BOOST_AUTO_TEST_CASE(ecrecoverFailedTest)
{
bytes params = *bcos::fromHexString(
Expand All @@ -61,6 +94,62 @@ BOOST_AUTO_TEST_CASE(ecrecoverFailedTest)
BOOST_CHECK(addressBytes.empty()); // but return empty
}


BOOST_AUTO_TEST_CASE(ecrecoverInvalidVTest)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001a" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b" // r
"01932751f4431c3b4c9d6fb1c826d138ee155ea72ac9013d66929f6a265386b4"); // s

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success); // also success
BOOST_CHECK(addressBytes.empty()); // but return empty
}

BOOST_AUTO_TEST_CASE(ecrecoverInvalidVTest2)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001d" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b" // r
"01932751f4431c3b4c9d6fb1c826d138ee155ea72ac9013d66929f6a265386b4"); // s

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success); // also success
BOOST_CHECK(addressBytes.empty()); // but return empty
}

BOOST_AUTO_TEST_CASE(ecrecoverInvalidInputLenTest)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001c" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b" // r
"01932751f4431c3b4c9d6fb1c826d138ee155ea72ac9013d66929f6a26538600"); // s

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success);
auto address =
*bcos::toHexString(addressBytes.data() + 12, addressBytes.data() + 12 + 20, "0x");
// 0x509eAd8B20064f21E35f920cB0c6d6cBC0C0Aa0d lower cases
BOOST_CHECK_EQUAL(address, "0x509ead8b20064f21e35f920cb0c6d6cbc0c0aa0d");
}

BOOST_AUTO_TEST_CASE(ecrecoverInvalidInputLenTest2)
{
bytes params = *bcos::fromHexString(
"aa0f7414b7f8648410f9818df3a1f43419d5c30313f430712033937ae57854c8" // hash
"000000000000000000000000000000000000000000000000000000000000001c" // v
"acd0d6c91242e514655815073f5f0e9aed671f68a4ed3e3e9d693095779f704b"); // s

auto [success, addressBytes] = crypto::ecRecover(ref(params));
BOOST_CHECK(success); // also success
BOOST_CHECK(addressBytes.empty()); // but return empty
}


BOOST_AUTO_TEST_SUITE_END()

} // namespace bcos::test
38 changes: 22 additions & 16 deletions bcos-framework/test/unittests/interfaces/FeaturesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,28 @@ BOOST_AUTO_TEST_CASE(feature)
BOOST_CHECK_EQUAL(features7.get("feature_balance_policy1"), true);

auto keys = Features::featureKeys();

BOOST_CHECK_EQUAL(keys.size(), 14);
BOOST_CHECK_EQUAL(keys[0], "bugfix_revert");
BOOST_CHECK_EQUAL(keys[1], "bugfix_statestorage_hash");
BOOST_CHECK_EQUAL(keys[2], "bugfix_evm_create2_delegatecall_staticcall_codecopy");
BOOST_CHECK_EQUAL(keys[3], "bugfix_event_log_order");
BOOST_CHECK_EQUAL(keys[4], "bugfix_call_noaddr_return");
BOOST_CHECK_EQUAL(keys[5], "bugfix_precompiled_codehash");
BOOST_CHECK_EQUAL(keys[6], "feature_dmc2serial");
BOOST_CHECK_EQUAL(keys[7], "feature_sharding");
BOOST_CHECK_EQUAL(keys[8], "feature_rpbft");
BOOST_CHECK_EQUAL(keys[9], "feature_paillier");
BOOST_CHECK_EQUAL(keys[10], "feature_balance");
BOOST_CHECK_EQUAL(keys[11], "feature_balance_precompiled");
BOOST_CHECK_EQUAL(keys[12], "feature_balance_policy1");
BOOST_CHECK_EQUAL(keys[13], "feature_paillier_add_raw");
// clang-format off
std::vector<std::string> compareKeys = {
"bugfix_revert",
"bugfix_statestorage_hash",
"bugfix_evm_create2_delegatecall_staticcall_codecopy",
"bugfix_event_log_order",
"bugfix_call_noaddr_return",
"bugfix_precompiled_codehash",
"feature_dmc2serial",
"feature_sharding",
"feature_rpbft",
"feature_paillier",
"feature_balance",
"feature_balance_precompiled",
"feature_balance_policy1",
"feature_paillier_add_raw"
};
// clang-format on
for (size_t i = 0; i < keys.size(); ++i)
{
BOOST_CHECK_EQUAL(keys[i], compareKeys[i]);
}
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 148f72c

Please sign in to comment.