Skip to content

Commit

Permalink
Fix xahau v1audit (XRPLF#250)
Browse files Browse the repository at this point in the history
  • Loading branch information
@RichardAH
RichardAH authored Dec 27, 2023
1 parent 97acfe9 commit 4ad6970
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 66 deletions.
53 changes: 24 additions & 29 deletions src/ripple/app/tx/impl/Escrow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,19 @@ EscrowFinish::preflight(PreflightContext const& ctx)
{
if (!ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
}

if (!ctx.tx.isFieldPresent(sfEscrowID) &&
!ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
if (ctx.tx.isFieldPresent(sfEscrowID) &&
ctx.tx.getFieldU32(sfOfferSequence) != 0)
return temMALFORMED;
}
else
{
if ((!ctx.tx.isFieldPresent(sfEscrowID) &&
!ctx.tx.isFieldPresent(sfOfferSequence)) ||
ctx.tx.isFieldPresent(sfEscrowID) &&
ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
}

return tesSUCCESS;
}
Expand Down Expand Up @@ -472,17 +480,6 @@ EscrowFinish::doApply()

bool const fixV1 = view().rules().enabled(fixXahauV1);

if (!fixV1)
{
if (escrowID && ctx_.tx[sfOfferSequence] != 0)
return temMALFORMED;
}
else
{
if (escrowID && offerSequence)
return temMALFORMED;
}

Keylet k = escrowID ? Keylet(ltESCROW, *escrowID)
: keylet::escrow(ctx_.tx[sfOwner], *offerSequence);

Expand Down Expand Up @@ -723,11 +720,19 @@ EscrowCancel::preflight(PreflightContext const& ctx)
{
if (!ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
}

if (!ctx.tx.isFieldPresent(sfEscrowID) &&
!ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
if (ctx.tx.isFieldPresent(sfEscrowID) &&
ctx.tx.getFieldU32(sfOfferSequence) != 0)
return temMALFORMED;
}
else
{
if ((!ctx.tx.isFieldPresent(sfEscrowID) &&
!ctx.tx.isFieldPresent(sfOfferSequence)) ||
ctx.tx.isFieldPresent(sfEscrowID) &&
ctx.tx.isFieldPresent(sfOfferSequence))
return temMALFORMED;
}

return preflight2(ctx);
}
Expand All @@ -744,16 +749,6 @@ EscrowCancel::doApply()
std::optional<std::uint32_t> offerSequence = ctx_.tx[~sfOfferSequence];

bool const fixV1 = view().rules().enabled(fixXahauV1);
if (!fixV1)
{
if (escrowID && ctx_.tx[sfOfferSequence] != 0)
return temMALFORMED;
}
else
{
if (escrowID && offerSequence)
return temMALFORMED;
}

Keylet k = escrowID ? Keylet(ltESCROW, *escrowID)
: keylet::escrow(ctx_.tx[sfOwner], *offerSequence);
Expand Down
21 changes: 11 additions & 10 deletions src/ripple/app/tx/impl/Import.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,16 +817,17 @@ Import::preflight(PreflightContext const& ctx)
<< " validation count: " << validationCount;

// check if the validation count is adequate
auto hasInsufficientQuorum = [&ctx](int quorum, int validationCount) {
if (ctx.rules.enabled(fixXahauV1))
{
return quorum > validationCount;
}
else
{
return quorum >= validationCount;
}
};
auto hasInsufficientQuorum =
[&ctx](uint64_t quorum, uint64_t validationCount) {
if (ctx.rules.enabled(fixXahauV1))
{
return quorum > validationCount;
}
else
{
return quorum >= validationCount;
}
};
if (hasInsufficientQuorum(quorum, validationCount))
{
JLOG(ctx.j.warn()) << "Import: xpop did not contain an 80% quorum for "
Expand Down
113 changes: 87 additions & 26 deletions src/ripple/app/tx/impl/URIToken.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ URIToken::preflight(PreflightContext const& ctx)
TER
URIToken::preclaim(PreclaimContext const& ctx)
{
bool const fixV1 = ctx.view.rules().enabled(fixXahauV1);

std::shared_ptr<SLE const> sleU;
uint32_t leFlags;
std::optional<AccountID> issuer;
Expand Down Expand Up @@ -180,6 +182,11 @@ URIToken::preclaim(PreclaimContext const& ctx)
AccountID const acc = ctx.tx.getAccountID(sfAccount);
uint16_t tt = ctx.tx.getFieldU16(sfTransactionType);

auto const sle =
ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount)));
if (!sle)
return tefINTERNAL;

switch (tt)
{
case ttURITOKEN_MINT: {
Expand Down Expand Up @@ -228,24 +235,75 @@ URIToken::preclaim(PreclaimContext const& ctx)
if (purchaseAmount < saleAmount)
return tecINSUFFICIENT_PAYMENT;

if (purchaseAmount.native() && saleAmount->native())
if (fixV1)
{
// if it's an xrp sale/purchase then no trustline needed
if (purchaseAmount >
(sleOwner->getFieldAmount(sfBalance) - ctx.tx[sfFee]))
return tecINSUFFICIENT_FUNDS;
}
if (purchaseAmount.native() && saleAmount->native())
{
// native transfer

// execution to here means it's an IOU sale
// check if the buyer has the right trustline with an adequate
// balance
STAmount needed{ctx.view.fees().accountReserve(
sle->getFieldU32(sfOwnerCount) + 1)};

STAmount availableFunds{accountFunds(
ctx.view, acc, purchaseAmount, fhZERO_IF_FROZEN, ctx.j)};
STAmount const fee = ctx.tx.getFieldAmount(sfFee).xrp();

if (purchaseAmount > availableFunds)
return tecINSUFFICIENT_FUNDS;
if (needed + fee < needed)
return tecINTERNAL;

needed += fee;

if (needed + purchaseAmount < needed)
return tecINTERNAL;

needed += purchaseAmount;

if (needed > sle->getFieldAmount(sfBalance))
return tecINSUFFICIENT_FUNDS;
}
else if (purchaseAmount.native() || saleAmount->native())
{
// should not be able to happen
return tecINTERNAL;
}
else
{
// iou transfer

STAmount availableFunds{accountFunds(
ctx.view,
acc,
purchaseAmount,
fhZERO_IF_FROZEN,
ctx.j)};

if (purchaseAmount > availableFunds)
return tecINSUFFICIENT_FUNDS;
}
}
else
{
// old logic

if (purchaseAmount.native() && saleAmount->native())
{
// if it's an xrp sale/purchase then no trustline needed
if (purchaseAmount >
(sleOwner->getFieldAmount(sfBalance) - ctx.tx[sfFee]))
return tecINSUFFICIENT_FUNDS;
}
else
{
// iou
STAmount availableFunds{accountFunds(
ctx.view,
acc,
purchaseAmount,
fhZERO_IF_FROZEN,
ctx.j)};

if (purchaseAmount > availableFunds)
return tecINSUFFICIENT_FUNDS;
}
}
return tesSUCCESS;
}

Expand Down Expand Up @@ -412,17 +470,6 @@ URIToken::doApply()
}

case ttURITOKEN_BUY: {
if (account_ == *owner)
{
// this is a clear operation
sleU->makeFieldAbsent(sfAmount);
if (sleU->isFieldPresent(sfDestination))
sleU->makeFieldAbsent(sfDestination);
sb.update(sleU);
sb.apply(ctx_.rawView());
return tesSUCCESS;
}

STAmount const purchaseAmount = ctx_.tx.getFieldAmount(sfAmount);

// check if the seller has listed it at all
Expand All @@ -446,8 +493,22 @@ URIToken::doApply()
// if it's an xrp sale/purchase then no trustline needed
if (purchaseAmount.native())
{
if (purchaseAmount >
((*sleOwner)[sfBalance] - ctx_.tx[sfFee]))
STAmount needed{sb.fees().accountReserve(
sle->getFieldU32(sfOwnerCount) + 1)};

STAmount const fee = ctx_.tx.getFieldAmount(sfFee).xrp();

if (needed + fee < needed)
return tecINTERNAL;

needed += fee;

if (needed + purchaseAmount < needed)
return tecINTERNAL;

needed += purchaseAmount;

if (needed > mPriorBalance)
return tecINSUFFICIENT_FUNDS;
}
else
Expand Down
5 changes: 4 additions & 1 deletion src/test/app/URIToken_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,10 @@ struct URIToken_test : public beast::unit_test::suite
using namespace std::literals::chrono_literals;

// setup env
Env env{*this, features};
Env env{
*this, envconfig(), features, nullptr, beast::severities::kWarning
// beast::severities::kTrace
};
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
Expand Down

0 comments on commit 4ad6970

Please sign in to comment.