Revert Lend - CaeraDenoir's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/V3Utils.sol#L98

Vulnerability details

Description

In the V3Utils.solcontract, the function executeWithPermit asks for the signature to be able to call permit to call execute with the token id of the owner. The problem lies in the lack of access checks on the execute function itself.

The idea behind having no security check in execute is V3Utils having no NFTs and no approvals by default, and the call via transform gives and removes the approval. The issue is that anyone can see a executeWithPermit call about to be made, take the signature, call it and then call execute with the token being approved by the permit. Since execute has no checks, any instruction can be passed after calling the permit. The same concept applies on the other functions in the contract, making anyone able to do anything that V3Utils.sol is able to do.

Impact

This vulnerability can lead to V3Utils.sol being used by an attacker to modify NFTs of anyone that tries to use it via permit, this being a potential loss of funds for regular users.

Proof Of Concept

Alice tries to use the V3Utils.sol contract to modify her Uniswapv3 position with a set of instructions.

V3Utils receives a executeWithPermit call with a set of instructions called instructionsAlice.

Bob sees the transaction that alice tries to do, and frontruns it.

nonfungiblePositionManager receives a permit call made by Bob, which allows the token of Alice to be managed by V3Utils.

V3Utils receives an execute call, with a set of instructions for Alice`s token called instructionsBob. It has no problem following the instructions since the NFT is approved in the V3Utils contract.


V3Utils receives a executeWithPermit call with a set of instructions called instructionsAlice. Bob`s instructions already happen, so this call could fail.

As demonstrated, bob can easily give instructions to Alice`s position, and could withdraw whatever is allowed.

Recommended mitigation steps

Add the owner requirement on every function. There should be no problems since the call done via transform sees V3Vault as the msg.sender, and is in fact the owner of the position.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L744

Vulnerability details

Summary

The liquidation can be prevented by the token owner without any problem. This would lead to the borrowers being unliquiditable.

Description

In the V3Vault.sol contract, the liquidate function has a major flaw. This is due to the owner of the NFT receiving the NFT via a safeTransfer which makes an external call the owner itself allowing him to revert the entire transaction(by reverting inside the onERC721Received function). The transfer itself happens on the _cleanupLoan function after removing the token from the owner in the lastTokenIndex and deleting the loan itself.

Impact

Since this would make the loans unliquiditable the protocol would not be used, since no one would want to lend money and not be able to recover it if the borrower does not wish to repay.

Proof Of Concept

Alice position state: Liquidatable.

Bob sees that Alice can be liquidated, and calls liquidate

Expected result:

Alice position state: Liquidated.
Bob is rewarded for calling liquidate.

Actual result if Alice wishes:

Alice position state: Liquidatable.
Bob is not rewarded since the liquidate call reverts.

Mitigation Steps

I would recommend to make the owner claim the NFT after a liquidation occurs.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blame/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L973

Vulnerability details

The full repayment can be prevented by anyone.

Description

The repay function in the V3Vault.sol contract has no minimum amount to be repayed, and no requirement for the borrower to be the one to repay the loan. This, combined with the _repay logic, that reverts if the function tries to pay more shares than in debt, makes it possible for anyone to prevent the full payment.

The idea behind it would be for a malicious user to repay 1 share of the loan debt right before the full repayment would occur, preventing the payment itself.

Impact

This issue can lead to borrowers being unable to cover a loan, and getting liquidated without being able to prevent it. And as such, would detract borrowers from using the protocol.

Proof Of Concept

Alice already has a position, based on WBTC/WETH. She notices both WETH and WBTC lose value quickly, and wishes to repay her loan to withdraw her liquidity.

Alice calls repay with amount equal to the entire amount she borrowed.

Bob, who sees what Alice is trying to do, and knows that by stalling a few blocks he can liquidate her, frontruns Alice transaction.

Bob calls repay with amount equal to the price of one share.

Expected transaction by alice:

Alice calls repay with amount equal to the entire amount she borrowed.

Real result of Alice`s transaction:

Repay reverts due to Alice trying to repay more shares than the loan debt.

Bob does this until Alice position is ready to be liquidated, then calls liquidate. Bob ends up receiving the penalty that is awarded for calling the liquidation.

Recommended Mitigation Steps

Do not revert if the repay function receives more shares to pay than the debt itself. Just pull the assets necessary to fully pay the debt and ignore the surplus.

Assessed type

Context