Overt Gunmetal Lizard
Medium
For the sake of simplicity, the issue will be described with the example of refinancing but the conditions for the issue to occur in affected code sections remain the same
When a borrower has accepted a loan offer, he has the ability to refinance() it : revise the terms of the initial loan offer.
This can be beneficial for the borrower in case the new proposal offers more advantageous interest rate or a longer borrowing period.
Doing so will result in closing the old loan and creating a new one. The fulfillment corresponding to the new offer will also be updated in the process.
The internal _refinance() function encapsulates the refinancing process.
function _refinance(
Refinancing calldata refinancing
) private returns (uint256 id, Loan memory newLoan, uint256 protocolFee) {
// snip --------------
@> _updateFulfillment(fulfillment, collateralAmountRequired, fulfillAmount, proposalId);
_transferLoanAmountAndProtocolFeeWithoutDeductingFromLoanAmount(proposal.from, loan.lender, debt, protocolFee);
_transferExcessCollateralIfAny(positionId, borrower, collateralAmountRequired, loan.collateralAmount);
id = nextLoanId;
@> _createLoan(id, proposal, positionId, proposal.from, borrower, collateralAmountRequired, fulfillAmount);
// snip --------------However, the resulting loan and fulfillment will contain values inflated by the protocol fees. The variable responsible for the inflation is fulfillAmount.
In order to create the corresponding loan and update the proposal's fulfillment, the _refinance() function calculates :
- the accumulated debt from the old loan
- the fees owed to the protocol
function _refinance(
Refinancing calldata refinancing
) private returns (uint256 id, Loan memory newLoan, uint256 protocolFee) {
// snip --------------
uint256 debt = _calculateDebt(loan.loanAmount, loan.interestRatePerSecond, block.timestamp - loan.startTime);
protocolFee = (debt * protocolFeeBasisPoints) / 10_000;
uint256 fulfillAmount = debt + protocolFee;
// snip --------------The sum of the debt and the protocol fees is stored in fulfillAmount. This variable is then used to perform multiple checks such as making sure :
- the new loan fills at least 10% of the corresponding proposal (or fills it entirely)
- the new loan won't exceeed the proposal's maximum loan amount
But it is also used to create the new loan and update the fulfillment (as seen in the Summary) which inflates the actual loan by the protocol's fees.
The protocol fee is an extra amount paid when a new loan is created to reward the protocol for helping lenders and borrowers to conclude a deal. This fee should not be involved in the deal's accounting.
An instance of the same bug can also be found in acceptLoanOfferAndFillOrder() in the relevant code sections :
An instance of the same bug can also be found in auction() in the relevant code sections :
As a general impact, the corresponding fulfillments and loans storage variables will return an inflated and incorrect value.
This can lead to different issues regarding both the lender and the borrower :
-
Lender : the excess amount taken by the protocol fees in the fulfillment will consume a part of the loan a lender will be able to offer to borrowers on the market. This means a lender will be able to lend less tokens than expected.
-
The loan the borrower will have to
repay()will be higher than expected as the function calculates the amount to repay using an inflated value.
function test_FeesInPositionAfterRefinance() public {
// protocol sets fees at 1%
vm.prank(owner);
predictDotLoan.updateProtocolFeeBasisPoints(100);
uint256 amountToLoan = 100 ether;
// creates a proposal signed by lender
IPredictDotLoan.Proposal memory proposal = _generateBaseProposal(IPredictDotLoan.QuestionType.Binary);
proposal.from = lender;
proposal.proposalType = IPredictDotLoan.ProposalType.LoanOffer;
(uint128 lendingNonce, ) = predictDotLoan.nonces(lender);
proposal.nonce = lendingNonce;
proposal.duration = 12 hours;
proposal.loanAmount = amountToLoan;
// the collateralization is 110% of the borrowed amount
proposal.collateralAmount = amountToLoan + ((amountToLoan * 10) / 100);
proposal.validUntil = vm.getBlockTimestamp() + 1 days;
proposal.questionId = questionId;
proposal.questionType = IPredictDotLoan.QuestionType.Binary;
proposal.outcome = true;
proposal.interestRatePerSecond = INTEREST_RATE_PER_SECOND;
proposal.salt = 42424242;
proposal.protocolFeeBasisPoints = _getProtocolFeeBasisPoints();
proposal.signature = _signProposal(proposal);
vm.prank(lender);
mockERC20.approve(address(predictDotLoan), type(uint256).max);
_mintCTF(borrower);
// hardcode the position ID of the CTF
uint256 positionId = 10670725991554937397181867830599699282910732402024524876672155713870914625521;
assertEq(mockERC20.balanceOf(lender), 700 ether);
assertEq(mockERC20.balanceOf(borrower), 0);
assertEq(mockCTF.balanceOf(lender, positionId), 0);
assertEq(mockCTF.balanceOf(borrower, positionId), 2000 ether);
// borrower will accept half of the loan offer
vm.startPrank(borrower);
uint256 acceptedLoan = 50 ether;
predictDotLoan.acceptLoanOffer(proposal, acceptedLoan);
vm.stopPrank();
// another lender creates another proposal with less collateral and an extended duration
proposal.from = lender2;
proposal.protocolFeeBasisPoints = _getProtocolFeeBasisPoints();
proposal.loanAmount = amountToLoan;
proposal.duration = 24 hours;
// this new proposal will require only 105% of collateralization
proposal.collateralAmount = amountToLoan + ((amountToLoan * 5) / 100);
proposal.signature = _signProposal(proposal, lender2PrivateKey);
vm.startPrank(lender2);
mockERC20.mint(lender2, proposal.loanAmount);
mockERC20.approve(address(predictDotLoan), type(uint256).max);
vm.stopPrank();
// the borrower wants to refinance its current loan to a more advantageous
vm.prank(borrower);
predictDotLoan.refinance(IPredictDotLoan.Refinancing(1, proposal));
// the fulfillment corresponding to the new proposal is read from storage
(bytes32 proposalId, uint256 collateral, uint256 loanAmountFromFulfillment) = predictDotLoan.getFulfillment(
proposal
);
// the borrower owns a certain a amount of LOAN_TOKEN but the new fulfillment includes an additional 1% (protocol fee)
assertNotEq(loanAmountFromFulfillment, mockERC20.balanceOf(borrower));
assertEq(loanAmountFromFulfillment, acceptedLoan + ((acceptedLoan * 1) / 100));
(
address newLoanBorrower,
address newLoanLender,
uint256 newLoanLoanPositionId,
uint256 newLoanCollateralAmount,
uint256 newLoanAmount,
uint256 newLoanInterestRatePerSecond,
uint256 newLoanStartTime,
uint256 newLoanMinimumDuration,
uint256 newLoanCallTime,
, // loanStatus
// questionType
) = predictDotLoan.loans(2); // new loan will has ID 2
assertNotEq(newLoanAmount, mockERC20.balanceOf(borrower));
assertEq(newLoanAmount, acceptedLoan + ((acceptedLoan * 1) / 100));
}As stated previously, the protocol fee is an extra amount paid to reward the protocol for helping lenders and borrowers to conclude a deal. This fee should not be involved in the deal's accounting.
Make sure the fulfillment and loan resulted in the operation does not include the protocol fee.