Go-langer - A control flow decision is made based on The block.timestamp environment variable.

[sherlock-admin]

Go-langer

medium

A control flow decision is made based on The block.timestamp environment variable.

Summary

A control flow decision is made based on The block.timestamp environment variable.

Vulnerability Detail

Someone i.e a validator can manipulate the block.timestamp variable in favour of themselves entering the auction. If a malicious validator Manipulates the block.timestamp, the attacker can liquidate the loan before the loan.endDate. The malicious actor could submit a bid for the collateral at a lower price than the fair market value due to the manipulated block.timestamp. By doing so, the actor could potentially win the auction and acquire the collateral for a lower price than what it is actually worth.

The malicious actor could submit a bid for the collateral at a higher price than the fair market value, but manipulate the block.timestamp to artificially extend the auction duration.

if (block.timestamp < loan.endDate) {
            revert CollateralIsNotLiquidableYet(loan.endDate, loanId);
        }

Impact

By doing so, the actor could prevent other bidders from winning the auction and acquire the collateral at a higher price than it is actually worth. The malicious actor could also enter the auction themselves and manipulate the block.timestamp to their advantage. They could do this by extending the auction duration or lowering the fair market value of the collateral to make it easier for them to win the auction.

Code Snippet

https://github.com/sherlock-audit/2023-02-kairos/blob/main/kairos-contracts/src/AuctionFacet.sol#L77

Tool used

Mythril Vs Code

Manual Review

Recommendation

For example, the smart contract could use a Chainlink oracle to obtain the current time instead of relying on the block.timestamp. This would make it more difficult for an attacker to manipulate the time since they would need to compromise both the blockchain protocol and the Chainlink oracle.

The other option is to introduce this refactored code to the useLoan function.

```require(block.number >= loan.liquidationBlockNumber + 20, "Not enough time has passed");```

function useLoan(BuyArg memory arg) internal {
        Loan storage loan = protocolStorage().loan[arg.loanId];

            require(block.number >= loan.liquidationBlockNumber + 20, "Not enough time has passed");

        checkLoanStatus(arg.loanId);
        uint256 toPay = price(arg.loanId);

        /* store as liquidated and paid before transfers to avoid malicious reentrency, following
        checks-effects-interaction pattern */
        loan.payment.liquidated = true;
        loan.payment.paid = toPay;
        loan.assetLent.checkedTransferFrom(msg.sender, address(this), toPay);
        loan.collateral.implem.safeTransferFrom(address(this), arg.to, loan.collateral.id);

        emit Buy(arg.loanId, abi.encode(arg));
    }