Skip to content

Commit

Permalink
ZIL-5447: Fix out-of-bounds iterator access
Browse files Browse the repository at this point in the history
  • Loading branch information
@JamesHinshelwood
JamesHinshelwood committed Nov 9, 2023
1 parent ab11eb8 commit 04852dd
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 31 deletions.
6 changes: 5 additions & 1 deletion src/libData/MiningData/DSPowSolution.cpp
View file
Expand Up @@ -30,6 +30,7 @@ DSPowSolution::DSPowSolution(const DSPowSolution& src)
m_nonce(src.m_nonce),
m_resultingHash(src.m_resultingHash),
m_mixHash(src.m_mixHash),
m_extraData(src.m_extraData),
m_lookupId(src.m_lookupId),
m_gasPrice(src.m_gasPrice),
m_govProposal(src.m_govProposal),
Expand Down Expand Up @@ -103,7 +104,9 @@ bool DSPowSolution::operator==(const DSPowSolution& sol) const {
(m_submitterPeer == sol.m_submitterPeer) &&
(m_submitterKey == sol.m_submitterKey) && (m_nonce == sol.m_nonce) &&
(m_resultingHash == sol.m_resultingHash) &&
(m_mixHash == sol.m_mixHash) && (m_lookupId == sol.m_lookupId) &&
(m_mixHash == sol.m_mixHash) &&
(m_extraData == sol.m_extraData) &&
(m_lookupId == sol.m_lookupId) &&
(m_gasPrice == sol.m_gasPrice) &&
(m_govProposal == sol.m_govProposal) &&
(m_signature == sol.m_signature));
Expand All @@ -117,6 +120,7 @@ DSPowSolution& DSPowSolution::operator=(const DSPowSolution& src) {
m_nonce = src.m_nonce;
m_resultingHash = src.m_resultingHash;
m_mixHash = src.m_mixHash;
m_extraData = src.m_extraData;
m_lookupId = src.m_lookupId;
m_gasPrice = src.m_gasPrice;
m_govProposal = src.m_govProposal;
Expand Down
28 changes: 20 additions & 8 deletions src/libMessage/Messenger.cpp
View file
Expand Up @@ -744,8 +744,11 @@ bool ProtobufToShardingStructureAnnouncement(
govProposalId = proto_member.powsoln().govdata().proposalid();
govVoteValue = proto_member.powsoln().govdata().votevalue();
}
// Copy 32 bytes of extraData at most. Validation of the PoW solution will fail later if the extraData was longer.
zbytes extraData(proto_member.powsoln().extradata().begin(), proto_member.powsoln().extradata().begin() + 32);
if (proto_member.powsoln().extradata().size() > 32) {
LOG_GENERAL(WARNING, "extra data is too large");
return false;
}
zbytes extraData(proto_member.powsoln().extradata().begin(), proto_member.powsoln().extradata().end());
allPoWs.emplace(
key, PoWSolution(proto_member.powsoln().nonce(), result, mixhash,
proto_member.powsoln().lookupid(), gasPrice,
Expand Down Expand Up @@ -1070,8 +1073,11 @@ bool ProtobufToDSPowSolution(const DSPoWSubmission& dsPowSubmission,
const uint64_t& nonce = dsPowSubmission.data().nonce();
const std::string& resultingHash = dsPowSubmission.data().resultinghash();
const std::string& mixHash = dsPowSubmission.data().mixhash();
// Copy 32 bytes of extraData at most. Validation of the PoW solution will fail later if the extraData was longer.
const zbytes extraData(dsPowSubmission.data().extradata().begin(), dsPowSubmission.data().extradata().begin() + 32);
if (dsPowSubmission.data().extradata().size() > 32) {
LOG_GENERAL(WARNING, "extra data is too large");
return false;
}
zbytes extraData(dsPowSubmission.data().extradata().begin(), dsPowSubmission.data().extradata().end());
const uint32_t& lookupId = dsPowSubmission.data().lookupid();
uint128_t gasPrice;
ProtobufByteArrayToNumber<uint128_t, UINT128_SIZE>(
Expand Down Expand Up @@ -2544,9 +2550,12 @@ bool Messenger::GetDSPoWSubmission(
nonce = result.data().nonce();
resultingHash = result.data().resultinghash();
mixHash = result.data().mixhash();
if (result.data().extradata().size() > 32) {
LOG_GENERAL(WARNING, "extra data is too large");
return false;
}
extraData.resize(result.data().extradata().size());
// Copy 32 bytes of extraData at most. Validation of the PoW solution will fail later if the extraData was longer.
std::copy(result.data().extradata().begin(), result.data().extradata().begin() + 32, extraData.begin());
std::copy(result.data().extradata().begin(), result.data().extradata().end(), extraData.begin());
lookupId = result.data().lookupid();
PROTOBUFBYTEARRAYTOSERIALIZABLE(result.signature(), signature);

Expand Down Expand Up @@ -2868,8 +2877,11 @@ bool Messenger::GetDSDSBlockAnnouncement(
govProposalId = protoDSWinnerPoW.powsoln().govdata().proposalid();
govVoteValue = protoDSWinnerPoW.powsoln().govdata().votevalue();
}
// Copy 32 bytes of extraData at most. Validation of the PoW solution will fail later if the extraData was longer.
zbytes extraData(protoDSWinnerPoW.powsoln().extradata().begin(), protoDSWinnerPoW.powsoln().extradata().begin() + 32);
if (protoDSWinnerPoW.powsoln().extradata().size() > 32) {
LOG_GENERAL(WARNING, "extra data is too large");
return false;
}
zbytes extraData(protoDSWinnerPoW.powsoln().extradata().begin(), protoDSWinnerPoW.powsoln().extradata().end());
dsWinnerPoWs.emplace(
key, PoWSolution(protoDSWinnerPoW.powsoln().nonce(), result, mixhash,
protoDSWinnerPoW.powsoln().lookupid(), gasPrice,
Expand Down
4 changes: 3 additions & 1 deletion tests/Data/Test_DSPowSolution.cpp
View file
Expand Up @@ -52,6 +52,7 @@ BOOST_AUTO_TEST_CASE(testDSPowSolutionClass) {
uint64_t nonceInput = TestUtils::DistUint64();
string resultingHashInput = TestUtils::GenerateRandomString(64);
string mixHashInput = TestUtils::GenerateRandomString(64);
zbytes extraDataInput = TestUtils::GenerateRandomCharVector(32);
uint32_t lookupIdInput = TestUtils::DistUint32();
uint128_t gasPriceInput = TestUtils::DistUint128();
uint32_t proposalIdInput = TestUtils::DistUint32();
Expand All @@ -61,7 +62,7 @@ BOOST_AUTO_TEST_CASE(testDSPowSolutionClass) {
dsps = DSPowSolution(
blockNumberInput, difficultyLevelInput, submitterPeerInput,
submitterKeyInput, nonceInput, resultingHashInput, mixHashInput,
lookupIdInput, gasPriceInput,
extraDataInput, lookupIdInput, gasPriceInput,
std::make_pair(proposalIdInput, voteValueInput), signatureInput);

DSPowSolution dsps2 = dsps;
Expand All @@ -74,6 +75,7 @@ BOOST_AUTO_TEST_CASE(testDSPowSolutionClass) {
BOOST_REQUIRE(dsps.GetNonce() == nonceInput);
BOOST_REQUIRE(dsps.GetResultingHash() == resultingHashInput);
BOOST_REQUIRE(dsps.GetMixHash() == mixHashInput);
BOOST_REQUIRE(dsps.GetExtraData() == extraDataInput);
BOOST_REQUIRE(dsps.GetLookupId() == lookupIdInput);
BOOST_REQUIRE(dsps.GetGasPrice() == gasPriceInput);
BOOST_REQUIRE(dsps.GetGovProposalId() == proposalIdInput);
Expand Down
40 changes: 23 additions & 17 deletions tests/POW/test_POW.cpp
View file
Expand Up @@ -276,17 +276,18 @@ BOOST_AUTO_TEST_CASE(mining_and_verification) {
// Light client mine and verify
uint8_t difficultyToUse = 5;
uint64_t blockToUse = 0;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, false,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
BOOST_REQUIRE(verifyLight);

rand1 = {{'0', '3'}};
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
bool verifyRand =
POWClient.PoWVerify(blockToUse, difficultyToUse, wrongHeaderHash,
winning_result.winning_nonce, winning_result.result,
Expand Down Expand Up @@ -319,17 +320,18 @@ BOOST_AUTO_TEST_CASE(mining_and_verification_big_block_number) {
// Light client mine and verify
uint8_t difficultyToUse = 3;
uint64_t blockToUse = 34567;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, false,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
BOOST_REQUIRE(verifyLight);

rand1 = {{'0', '3'}};
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
bool verifyRand =
POWClient.PoWVerify(blockToUse, difficultyToUse, wrongHeaderHash,
winning_result.winning_nonce, winning_result.result,
Expand Down Expand Up @@ -362,17 +364,18 @@ BOOST_AUTO_TEST_CASE(mining_and_verification_full) {
// Light client mine and verify
uint8_t difficultyToUse = 5;
uint64_t blockToUse = 0;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, true,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
BOOST_REQUIRE(verifyLight);

rand1 = {{'0', '3'}};
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
bool verifyRand =
POWClient.PoWVerify(blockToUse, difficultyToUse, wrongHeaderHash,
winning_result.winning_nonce, winning_result.result,
Expand Down Expand Up @@ -405,10 +408,11 @@ BOOST_AUTO_TEST_CASE(mining_high_diffculty_time_out) {
// Light client mine and verify
uint8_t difficultyToUse = 50;
uint64_t blockToUse = 0;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, true,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
BOOST_REQUIRE(!winning_result.success);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
Expand Down Expand Up @@ -441,17 +445,18 @@ BOOST_AUTO_TEST_CASE(gpu_mining_and_verification_1) {
// Light client mine and verify
uint8_t difficultyToUse = 10;
uint64_t blockToUse = 0;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, true,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
BOOST_REQUIRE(verifyLight);

rand1 = {{'0', '3'}};
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
bool verifyRand =
POWClient.PoWVerify(blockToUse, difficultyToUse, wrongHeaderHash,
winning_result.winning_nonce, winning_result.result,
Expand Down Expand Up @@ -498,17 +503,18 @@ BOOST_AUTO_TEST_CASE(gpu_mining_and_verification_2) {
// Light client mine and verify
uint8_t difficultyToUse = 20;
uint64_t blockToUse = 1234567;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result =
POWClient.PoWMine(blockToUse, difficultyToUse, keyPair, headerHash, true,
std::time(0), POW_WINDOW_IN_SECONDS);
std::time(0), POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
BOOST_REQUIRE(verifyLight);

rand1 = {{'0', '3'}};
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto wrongHeaderHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, {});
bool verifyRand =
POWClient.PoWVerify(blockToUse, difficultyToUse, wrongHeaderHash,
winning_result.winning_nonce, winning_result.result,
Expand Down
10 changes: 6 additions & 4 deletions tests/POW/test_RemoteMine.cpp
View file
Expand Up @@ -53,15 +53,17 @@ void TestRemoteMineCase_1() {
PubKey pubKey(privKey);
std::cout << "Test with pubkey: " << pubKey << std::endl;
PairOfKey keyPair(privKey, pubKey);
zbytes extraData = TestUtils::GenerateRandomCharVector(32);

// Light client mine and verify
uint8_t difficultyToUse = POW_DIFFICULTY;
uint64_t blockToUse = 1000;
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0);
auto headerHash = POW::GenHeaderHash(rand1, rand2, peer, pubKey, 0, 0, extraData);
auto boundary = POW::DifficultyLevelInIntDevided(difficultyToUse);

HeaderHashParams headerParams{rand1, rand2, peer, pubKey, 0, 0};
ethash_mining_result_t winning_result = POWClient.RemoteMine(
keyPair, blockToUse, headerHash, boundary, POW_WINDOW_IN_SECONDS);
keyPair, blockToUse, headerHash, boundary, POW_WINDOW_IN_SECONDS, headerParams);
bool verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
Expand All @@ -72,7 +74,7 @@ void TestRemoteMineCase_1() {
boundary = POW::DifficultyLevelInIntDevided(difficultyToUse);

winning_result = POWClient.RemoteMine(keyPair, blockToUse, headerHash,
boundary, POW_WINDOW_IN_SECONDS);
boundary, POW_WINDOW_IN_SECONDS, headerParams);
verifyLight = POWClient.PoWVerify(
blockToUse, difficultyToUse, headerHash, winning_result.winning_nonce,
winning_result.result, winning_result.mix_hash);
Expand All @@ -84,4 +86,4 @@ int main([[gnu::unused]] int argc, [[gnu::unused]] const char* argv[]) {
INIT_STDOUT_LOGGER();

TestRemoteMineCase_1();
}
}

0 comments on commit 04852dd

Please sign in to comment.