Skip to content
Open
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
48 changes: 44 additions & 4 deletions include/xrpl/tx/invariants/LoanInvariant.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,51 @@ namespace xrpl {
*
* 1. If `Loan.PaymentRemaining = 0` then `Loan.PrincipalOutstanding = 0`
*
* A loan may only be deleted once it is fully paid off (no payments
* remaining):
* The following invariants only apply once the `LendingProtocolV1_1`
* amendment is enabled (the two-step "pending loan" flow):
*
* 2. A loan may only be deleted by a `LoanDelete` transaction.
* 3. A loan that is not fully paid off must not be deleted.
* 2. A loan's `OwnerNode` may only be added, removed, or changed on an
* existing loan by `LoanAccept`.
* 3. A loan's `lsfLoanPending` flag may only be cleared (never set) on an
* existing loan, and only by `LoanAccept`.
* 4. A `LoanAccept` may only modify an existing loan when:
* a. the loan was pending (`lsfLoanPending` set);
* b. the submitting account (`Account`) is the loan's `Borrower`;
* c. the loan's `StartDate` is still in the future.
* 5. A `LoanSet` that creates a loan must use exactly one of two mutually
* exclusive creation paths: it either names a `Borrower`, or it carries a
* a `CounterpartySignature`. Specifically:
* a. If `Borrower` is present, `Counterparty` and `CounterpartySignature`
* must be absent.
* b. If `CounterpartySignature` is present, `Borrower` must be absent.
* c. Either `Borrower`, or `CounterpartySignature`,
* must be present.
* d. If `Borrower` is present, it must differ from the submitting
* `Account`.
* 6. A `LoanSet` that creates a loan must set `lsfLoanPending` if and only if it
* starts the two-step flow, i.e. `Borrower` and `StartDate` are present while
* `Counterparty` and `CounterpartySignature` are absent.
* 7. A `LoanSet` that creates a loan must set the loan's `Borrower`.
* Additionally, a pending loan's `StartDate` must be in the future (so it is
* still in the future when `LoanAccept` finalizes it).
* 8. A pending loan (`lsfLoanPending` set) must not be linked into the
* borrower's directory (`OwnerNode` absent), and a non-pending loan must
* be linked (`OwnerNode` present).
*
* While the `LendingProtocolV1_1` amendment is not enabled, a `LoanSet` that
* creates a loan must not use any of the two-step flow's inputs:
*
* 9. It must not create a pending loan (`lsfLoanPending` must be clear).
* 10. It must not be given a `Borrower`.
* 11. It must always carry a `CounterpartySignature`.
*
* A loan may only be deleted once it is fully paid off (no payments remaining)
* or, once the `LendingProtocolV1_1` amendment is enabled, while it is still
* pending:
*
* 12. A loan may only be deleted by a `LoanDelete` transaction.
* 13. A pending loan must not be deleted while the amendment is not enabled.
* 14. A loan that is neither fully paid off nor pending must not be deleted.
*
*/
class ValidLoan
Expand Down
5 changes: 5 additions & 0 deletions include/xrpl/tx/invariants/VaultInvariant.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ namespace xrpl {
* verifies the payment was split correctly between principal and interest
* - no vault transaction can change loss unrealized (it's updated by loan
* transactions)
* - AssetsReserved (principal held back for pending loans) may only be changed
* by LoanDelete, LoanAccept, or a LoanSet that creates a pending loan
* (Borrower present, CounterpartySignature absent)
*
*/
class ValidVault
Expand All @@ -71,6 +74,7 @@ class ValidVault
Number assetsAvailable = 0;
Number assetsMaximum = 0;
Number lossUnrealized = 0;
Number assetsReserved = 0;
std::uint8_t withdrawalPolicy = 0;
std::uint8_t scale = 0;

Expand All @@ -92,6 +96,7 @@ class ValidVault
Number principalOutstanding = 0;
Number totalValueOutstanding = 0;
Number managementFeeOutstanding = 0;
uint32_t flags = 0;

// Interest booked to the vault at loan creation: the portion of the
// total value owed that is neither principal nor broker management fee.
Expand Down
234 changes: 227 additions & 7 deletions src/libxrpl/tx/invariants/LoanInvariant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STLedgerEntry.h>
Expand Down Expand Up @@ -33,7 +34,7 @@ ValidLoan::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after
bool
ValidLoan::finalize(
STTx const& tx,
TER const,
TER const result,
XRPAmount const,
ReadView const& view,
beast::Journal const& j)
Expand All @@ -42,6 +43,8 @@ ValidLoan::finalize(
// is not enabled, so there's no need to check it.

auto const txType = tx.getTxnType();
// The pending-loan checks below only apply once the two-step flow exists.
bool const lendingV11Enabled = view.rules().enabled(featureLendingProtocolV1_1);

for (auto const& [before, after] : loans_)
{
Expand Down Expand Up @@ -71,6 +74,192 @@ ValidLoan::finalize(
JLOG(j.fatal()) << "Invariant failed: Loan Overpayment flag changed";
return false;
}
if (before)
{
// The lsfLoanPending flag may only be cleared (finalising the
// loan), and only by LoanAccept. It must never be set on an
// existing loan.
bool const wasPending = before->isFlag(lsfLoanPending);
bool const isPending = after->isFlag(lsfLoanPending);

if (!lendingV11Enabled && (wasPending || isPending))
{
JLOG(j.fatal()) << "Invariant failed: Loan Pending flag changed "
"when the amendment is not enabled";
return false;
}

if (wasPending != isPending &&
(!lendingV11Enabled || txType != ttLOAN_ACCEPT || (!wasPending && isPending)))
{
JLOG(j.fatal()) << "Invariant failed: Loan Pending flag changed "
"by an unauthorized transaction";
return false;
}

// LoanAccept may only process a loan that was pending.
if (txType == ttLOAN_ACCEPT && !wasPending)
{
JLOG(j.fatal()) << "Invariant failed: LoanAccept modified a "
"Loan that was not pending";
return false;
}

// The OwnerNode may only be added to an existing loan, and only by
// LoanAccept (which links the loan into the borrower's directory
// once accepted). It must never be removed or changed.
bool const beforeHasNode = before->isFieldPresent(sfOwnerNode);
bool const afterHasNode = after->isFieldPresent(sfOwnerNode);
if (beforeHasNode &&
(!afterHasNode ||
before->getFieldU64(sfOwnerNode) != after->getFieldU64(sfOwnerNode)))
{
JLOG(j.fatal()) << "Invariant failed: Loan OwnerNode removed "
"or changed";
return false;
}
if (!beforeHasNode && afterHasNode && (!lendingV11Enabled || txType != ttLOAN_ACCEPT))
{
JLOG(j.fatal()) << "Invariant failed: Loan OwnerNode added "
"by an unauthorized transaction";
return false;
}

// LoanAccept must be submitted by the loan's Borrower, and may only
// finalise a loan whose StartDate is still in the future.
if (lendingV11Enabled && txType == ttLOAN_ACCEPT)
{
if (after->getAccountID(sfBorrower) != tx.getAccountID(sfAccount))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: LOW

The Borrower authorization check reads from after (post-modification state) instead of before (pre-modification state). A buggy or malicious transactor that overwrites sfBorrower to match the submitter during execution would bypass this invariant, defeating its purpose as a last-line-of-defense authorization check.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Change after->getAccountID(sfBorrower) to before->getAccountID(sfBorrower) on line 132. The invariant check should read the sfBorrower field from the pre-modification state (before) rather than the post-modification state (after). This ensures that even if a buggy transactor overwrites sfBorrower during execution to match the submitter's account, the invariant will still catch the unauthorized access by comparing against the original, unmodified value. The before variable is already available in scope (the code is inside the if (before) block starting at line 77).

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change
if (after->getAccountID(sfBorrower) != tx.getAccountID(sfAccount))
if (before->getAccountID(sfBorrower) != tx.getAccountID(sfAccount))

{
JLOG(j.fatal()) << "Invariant failed: LoanAccept submitted "
"by an account other than the Borrower";
return false;
}
if (after->getFieldU32(sfStartDate) <=
view.parentCloseTime().time_since_epoch().count())
{
JLOG(j.fatal()) << "Invariant failed: LoanAccept processed a "
"Loan whose StartDate is not in the future";
return false;
}
}
}
// On creation, LoanSet must use exactly one of two mutually exclusive
// paths: it either names a Borrower with a StartDate (starting the
// two-step flow) or carries a CounterpartySignature. A Borrower with a
// StartDate and no CounterpartySignature or Counterparty starts the
// two-step flow and must create a pending loan; any other LoanSet must
// create an active (non-pending) loan.
if (isTesSuccess(result))
{
if (lendingV11Enabled && !before && txType == ttLOAN_SET)
{
bool const hasBorrower = tx.isFieldPresent(sfBorrower);
bool const hasCounterpartySig = tx.isFieldPresent(sfCounterpartySignature);
bool const hasStartDate = tx.isFieldPresent(sfStartDate);
bool const hasCounterparty = tx.isFieldPresent(sfCounterparty);
bool isTwoStepFlow = hasBorrower && hasStartDate;
bool isOneStepFlow = hasCounterpartySig;

if (!isTwoStepFlow && !isOneStepFlow)
{
JLOG(j.fatal()) << "Invariant failed: LoanSet specified neither "
"a Borrower with a StartDate nor a CounterpartySignature";
return false;
}

if (isTwoStepFlow && isOneStepFlow)
{
JLOG(j.fatal()) << "Invariant failed: LoanSet specified both "
"a Borrower with a StartDate and a CounterpartySignature";
return false;
}

if (isOneStepFlow && hasBorrower)
{
JLOG(j.fatal()) << "Invariant failed: LoanSet specified a "
"Borrower with a CounterpartySignature";
return false;
}

if (isTwoStepFlow && hasCounterparty)
{
JLOG(j.fatal()) << "Invariant failed: LoanSet specified a "
"Borrower with a StartDate and a Counterparty";
return false;
}

// In the two-step flow the named Borrower must be a different
// account from the one submitting the LoanSet.
if (hasBorrower && tx.getAccountID(sfBorrower) == tx.getAccountID(sfAccount))
{
JLOG(j.fatal()) << "Invariant failed: LoanSet Borrower is the "
"submitting account";
return false;
}

// The two-step flow is started (and the loan created pending) when
// the LoanSet names a Borrower and a StartDate but carries neither a
// Counterparty nor a CounterpartySignature.
bool const shouldPend =
hasBorrower && hasStartDate && !hasCounterpartySig && !hasCounterparty;
if (shouldPend != after->isFlag(lsfLoanPending))
{
JLOG(j.fatal()) << "Invariant failed: LoanSet pending flag does "
"not match the two-step flow inputs";
return false;
}

// A pending loan (the two-step flow) is accepted in a later ledger,
// so its StartDate must be strictly in the future at creation to
// remain in the future when LoanAccept finalizes it.
if (shouldPend &&
after->getFieldU32(sfStartDate) <=
view.parentCloseTime().time_since_epoch().count())
{
JLOG(j.fatal()) << "Invariant failed: LoanSet created a pending "
"Loan whose StartDate is not in the future";
return false;
}

// A created loan must always record its Borrower. As sfBorrower is a
// required field it is always present (defaulting to the zero
// account), so a loan whose Borrower was never set carries the zero
// account.
if (!after->isFieldPresent(sfBorrower) ||
after->getAccountID(sfBorrower) == beast::kZero)
{
JLOG(j.fatal()) << "Invariant failed: LoanSet did not set the "
"Loan Borrower";
return false;
}
}
// Without the two-step flow amendment, LoanSet must not make use of any
// of its inputs: it must not create a pending loan, must not be given a
// Borrower, and must always carry a CounterpartySignature.
if (!lendingV11Enabled && !before && txType == ttLOAN_SET)
{
if (after->isFlag(lsfLoanPending))
{
JLOG(j.fatal()) << "Invariant failed: LoanSet set the Loan "
"Pending flag when the amendment is not enabled";
return false;
}
if (tx.isFieldPresent(sfBorrower))
{
JLOG(j.fatal()) << "Invariant failed: LoanSet specified a "
"Borrower when the amendment is not enabled";
return false;
}
if (!tx.isFieldPresent(sfCounterpartySignature))
{
JLOG(j.fatal()) << "Invariant failed: LoanSet omitted the "
"CounterpartySignature when the amendment is "
"not enabled";
return false;
}
}
}
// Must not be negative - STNumber
for (auto const field :
{&sfLoanServiceFee,
Expand Down Expand Up @@ -107,10 +296,30 @@ ValidLoan::finalize(
return false;
}
}
// A pending loan must not be linked into the borrower's directory, and
// a non-pending loan must be linked.
if (lendingV11Enabled)
{
bool const isPending = after->isFlag(lsfLoanPending);
bool const hasNode = after->isFieldPresent(sfOwnerNode);
if (isPending && hasNode)
{
JLOG(j.fatal()) << "Invariant failed: pending Loan is linked "
"into the borrower's directory";
return false;
}
if (!isPending && !hasNode)
{
JLOG(j.fatal()) << "Invariant failed: active Loan is not linked "
"into the borrower's directory";
return false;
}
}
}

// A loan may only be deleted by a LoanDelete transaction, and only once it
// is fully paid off (no payments remaining). Deleting a loan with
// is fully paid off (no payments remaining) or, once the two-step flow
// exists, while it is still pending acceptance. Deleting an active loan with
// outstanding obligations is a violation.
for (auto const& loan : deletedLoans_)
{
Expand All @@ -121,13 +330,24 @@ ValidLoan::finalize(
return false;
}

if (loan->at(sfPaymentRemaining) != 0 ||
loan->at(sfTotalValueOutstanding) != beast::kZero ||
loan->at(sfPrincipalOutstanding) != beast::kZero ||
loan->at(sfManagementFeeOutstanding) != beast::kZero)
bool const wasPending = loan->isFlag(lsfLoanPending);
// A pending loan cannot exist while the amendment is not enabled.
if (wasPending && !lendingV11Enabled)
{
JLOG(j.fatal()) << "Invariant failed: pending Loan deleted when the "
"amendment is not enabled";
return false;
}

bool const paidOff = loan->at(sfPaymentRemaining) == 0 &&
loan->at(sfTotalValueOutstanding) == beast::kZero &&
loan->at(sfPrincipalOutstanding) == beast::kZero &&
loan->at(sfManagementFeeOutstanding) == beast::kZero;
bool const pending = lendingV11Enabled && wasPending;
if (!paidOff && !pending)
{
JLOG(j.fatal()) << "Invariant failed: Loan deleted while not fully "
"paid off";
"paid off and not pending";
return false;
}
}
Expand Down
Loading
Loading