Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/herder/TxSetFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "util/ProtocolVersion.h"
#include "util/XDRCereal.h"
#include "util/XDROperators.h"
#include "util/numeric.h"
#include "xdrpp/marshal.h"

#include <Tracy.hpp>
Expand Down Expand Up @@ -2277,7 +2278,8 @@ ApplicableTxSetFrame::getTotalFees(LedgerHeader const& lh) const
{
for (auto const& tx : phaseTxs)
{
total += tx->getFee(lh, getTxBaseFee(tx), true);
total =
saturatingAdd(total, tx->getFee(lh, getTxBaseFee(tx), true));
}
}
return total;
Expand All @@ -2292,7 +2294,7 @@ ApplicableTxSetFrame::getTotalInclusionFees() const
{
for (auto const& tx : phaseTxs)
{
total += tx->getInclusionFee();
total = saturatingAdd(total, tx->getInclusionFee());
}
}
return total;
Expand Down
5 changes: 3 additions & 2 deletions src/herder/TxSetFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -508,11 +508,12 @@ class ApplicableTxSetFrame
return mPhases.size();
}

// Returns the sum of all fees that this transaction set would take.
// Returns the sum of all fees that this transaction set would take. Clamps
// at INT64_MAX.
int64_t getTotalFees(LedgerHeader const& lh) const;

// Returns the sum of all _inclusion fee_ bids for all transactions in this
// set.
// set. Clamps at INT64_MAX.
int64_t getTotalInclusionFees() const;

// Returns whether this transaction set is generalized, i.e. representable
Expand Down
69 changes: 69 additions & 0 deletions src/herder/test/TxSetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
#include "util/Math.h"
#include "util/ProtocolVersion.h"
#include "util/XDRCereal.h"
#include "util/numeric.h"
Comment thread
graydon marked this conversation as resolved.
#include <algorithm>
#include <limits>
#include <map>
namespace stellar
{
Expand Down Expand Up @@ -1782,6 +1784,73 @@ TEST_CASE("generalized tx set with multiple txs per source account",
}
}

TEST_CASE("tx set fee totals saturate on overflow", "[txset]")
{
VirtualClock clock;
auto cfg = getTestConfig();
Application::pointer app = createTestApplication(clock, cfg);
auto root = app->getRoot();

auto minBalance = app->getLedgerManager().getLastMinBalance(0);
auto a = root->create("fee-total-a", 2 * minBalance);
auto b = root->create("fee-total-b", 2 * minBalance);
auto c = root->create("fee-total-c", 2 * minBalance);
auto innerA = a.tx({payment(a.getPublicKey(), 1)});
auto innerB = b.tx({payment(b.getPublicKey(), 1)});
auto innerC = c.tx({payment(c.getPublicKey(), 1)});

auto const& lcl = app->getLedgerManager().getLastClosedLedgerHeader();

int64_t constexpr maxClamp = std::numeric_limits<int64_t>::max();

auto makeSet = [&](std::vector<TransactionFrameBasePtr> const& txs) {
testtxset::PhaseComponents classicPhase;
classicPhase.emplace_back(std::nullopt, txs);
std::vector<testtxset::PhaseComponents> phases;
phases.emplace_back(std::move(classicPhase));
phases.emplace_back();
return testtxset::makeNonValidatedGeneralizedTxSet(phases, *app,
lcl.hash);
};
auto bump = [&](TransactionFrameBaseConstPtr inner, int64_t fee) {
return feeBump(*app, *root, inner, fee,
/* useInclusionAsFullFee */ true);
};

SECTION("exact when the total does not overflow")
{
// 2 * (INT64_MAX / 2) is INT64_MAX - 1, which should not clamp
auto [txSet, applicable] =
makeSet({bump(innerA, maxClamp / 2), bump(innerB, maxClamp / 2)});
REQUIRE(applicable);
REQUIRE(applicable->getTotalInclusionFees() == maxClamp - 1);
REQUIRE(applicable->getTotalFees(lcl.header) == maxClamp - 1);
}

SECTION("saturates at the overflow boundary")
{
// Fees total to INT64_MAX + 1, so this should clamp
auto [txSet, applicable] = makeSet(
{bump(innerA, maxClamp / 2 + 1), bump(innerB, maxClamp / 2 + 1)});
REQUIRE(applicable);
REQUIRE(applicable->sizeTxTotal() == 2);
REQUIRE(applicable->getTotalInclusionFees() == maxClamp);
REQUIRE(applicable->getTotalFees(lcl.header) == maxClamp);
}

SECTION("saturates far past the overflow boundary")
{
// Three INT64_MAX fees should still clamp to INT64_MAX
auto [txSet, applicable] =
makeSet({bump(innerA, maxClamp), bump(innerB, maxClamp),
bump(innerC, maxClamp)});
REQUIRE(applicable);
REQUIRE(applicable->sizeTxTotal() == 3);
REQUIRE(applicable->getTotalInclusionFees() == maxClamp);
REQUIRE(applicable->getTotalFees(lcl.header) == maxClamp);
}
}

TEST_CASE("generalized tx set fees", "[txset][soroban]")
{
VirtualClock clock;
Expand Down
31 changes: 26 additions & 5 deletions src/overlay/Peer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ CapacityTrackedMessage::CapacityTrackedMessage(std::weak_ptr<Peer> peer,
// Whether to check transaction signatures in the background, adding them to
// the signature cache in the process.
bool const checkTxSig =
self->isAuthenticatedAtomic() &&
self->mAppConnector.getConfig().BACKGROUND_TX_SIG_VERIFICATION &&
self->useBackgroundThread();

Expand Down Expand Up @@ -1419,7 +1420,8 @@ Peer::recvDontHave(StellarMessage const& msg)
}

bool
Peer::process(QueryInfo& queryInfo, std::optional<uint32_t> maxQueriesPerWindow)
Peer::process(QueryInfo& queryInfo, std::optional<Hash> queryKey,
std::optional<uint32_t> maxQueriesPerWindow)
{
auto const& cfg = mAppConnector.getConfig();
std::chrono::seconds const QUERY_WINDOW =
Expand All @@ -1432,8 +1434,27 @@ Peer::process(QueryInfo& queryInfo, std::optional<uint32_t> maxQueriesPerWindow)
{
queryInfo.mLastTimeStamp = mAppConnector.now();
queryInfo.mNumQueries = 0;
queryInfo.mRequestedObjects.clear();
}
return queryInfo.mNumQueries < QUERIES_PER_WINDOW;
// NB: check the rate _before_ the table, to cap table size.
if (queryInfo.mNumQueries < QUERIES_PER_WINDOW)
{
if (queryKey.has_value())
{
auto [it, _] =
queryInfo.mRequestedObjects.try_emplace(queryKey.value(), 0);
if (it->second < QUERY_RESPONSE_MULTIPLIER)
Comment thread
graydon marked this conversation as resolved.
{
it->second++;
return true;
}
}
else
{
return true;
}
}
return false;
}

#ifdef BUILD_TESTS
Expand All @@ -1460,7 +1481,7 @@ Peer::recvGetTxSet(StellarMessage const& msg)
{
ZoneScoped;
releaseAssert(threadIsMain());
if (!process(mTxSetQueryInfo))
if (!process(mTxSetQueryInfo, msg.txSetHash()))
{
return;
}
Expand Down Expand Up @@ -1601,7 +1622,7 @@ Peer::recvGetSCPQuorumSet(StellarMessage const& msg)
{
ZoneScoped;
releaseAssert(threadIsMain());
if (!process(mQSetQueryInfo))
if (!process(mQSetQueryInfo, msg.qSetHash()))
{
return;
}
Expand Down Expand Up @@ -1685,7 +1706,7 @@ Peer::recvGetSCPState(StellarMessage const& msg)
{
ZoneScoped;
releaseAssert(threadIsMain());
if (!process(mSCPStateQueryInfo, GET_SCP_STATE_MAX_RATE))
if (!process(mSCPStateQueryInfo, std::nullopt, GET_SCP_STATE_MAX_RATE))
{
CLOG_DEBUG(Overlay, "Dropping GET_SCP_STATE request from {}",
KeyUtils::toShortString(mPeerID));
Expand Down
2 changes: 2 additions & 0 deletions src/overlay/Peer.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class Peer : public std::enable_shared_from_this<Peer>,
{
VirtualClock::time_point mLastTimeStamp;
uint32_t mNumQueries{0};
std::unordered_map<Hash, size_t> mRequestedObjects;
};

static inline int
Expand Down Expand Up @@ -328,6 +329,7 @@ class Peer : public std::enable_shared_from_this<Peer>,
// optionally set `maxQueriesPerWindow` to override the default per-window
// query limit.
bool process(QueryInfo& queryInfo,
std::optional<Hash> queryKey = std::nullopt,
std::optional<uint32_t> maxQueriesPerWindow = std::nullopt);

void recvMessage(std::shared_ptr<CapacityTrackedMessage> msgTracker);
Expand Down
60 changes: 60 additions & 0 deletions src/overlay/test/OverlayTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3665,4 +3665,64 @@ TEST_CASE("populateSignatureCache tests", "[overlay]")
REQUIRE(misses == 1);
}
}

TEST_CASE(
"unauthenticated peer cannot trigger background signature verification",
"[overlay]")
{
VirtualClock clock;
auto cfg = getTestConfig();
// `PeerDoor` only listens when RUN_STANDALONE is false
cfg.RUN_STANDALONE = false;
auto app = createTestApplication(clock, cfg);

// Source the transaction from root
auto root = app->getRoot();
auto tx = root->tx({txtest::payment(root->getPublicKey(), 1)});

AuthenticatedMessage message;
message.v0().message.type(TRANSACTION);
message.v0().message.transaction() = tx->getEnvelope();
auto record = xdr::xdr_to_msg(message);

// Connect without sending HELLO, so the peer stays below GOT_HELLO for as
// long as we need it to.
asio::io_context rawIOContext;
asio::ip::tcp::socket rawSocket(rawIOContext);
rawSocket.connect(asio::ip::tcp::endpoint(
asio::ip::address::from_string("127.0.0.1"), cfg.PEER_PORT));
testutil::crankUntil(
app,
[&]() {
return app->getOverlayManager().getInboundPendingPeers().size() ==
1;
},
std::chrono::seconds(5));

auto peer = app->getOverlayManager().getInboundPendingPeers().front();
REQUIRE_FALSE(peer->isAuthenticatedForTesting());

PubKeyUtils::clearVerifySigCache();
uint64_t hits = 0;
uint64_t misses = 0;
PubKeyUtils::flushVerifySigCacheCounts(hits, misses);

// Send the transaction while unauthenticated
asio::write(rawSocket,
asio::buffer(record->raw_data(), record->raw_size()));

// Peer should be dropped for sending a TRANSACTION before the handshake
testutil::crankUntil(
app, [&]() { return !peer->isConnectedForTesting(); },
std::chrono::seconds(5));
REQUIRE(peer->getDropReason() ==
"received TRANSACTION before completed handshake");

// Must not have been any signature verifications performed
PubKeyUtils::flushVerifySigCacheCounts(hits, misses);
REQUIRE(hits + misses == 0);

asio::error_code ec;
rawSocket.close(ec);
}
}
2 changes: 0 additions & 2 deletions src/scp/test/SCPTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3483,7 +3483,6 @@ TEST_CASE("nomination tests core5", "[scp][nominationprotocol]")
}
}

#ifdef CAP_0087
TEST_CASE("nomination times out structurally-valid value into empty tx set",
"[scp][nomination]")
{
Expand Down Expand Up @@ -3937,6 +3936,5 @@ TEST_CASE("incoming PREPARE with non-tx-set-invalid value is dropped",
// No local emit triggered.
REQUIRE(scp.mEnvs.empty());
}
#endif // CAP_0087

}
Loading