code-423n4 / 2023-03-mute-findings

2 stars 1 forks source link

Params of Lien struct are not emitted when lien is created making it difficult to track #30

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-contest225/blob/af270a1fb366c8dda0fbbdd7d6ddaf6f0011992f/contracts/Core.sol#L306 https://github.com/code-423n4/2023-03-contest225/blob/af270a1fb366c8dda0fbbdd7d6ddaf6f0011992f/contracts/Core.sol#L188

Vulnerability details

Impact

Protocol does not store any information about Lien. When users want to interact, they have to send the whole Lien struct along with lienId, and the protocol will verify if this data is correct by hash.

This approach reduces onchain storage and can save a lot of gas. However, it also creates difficulty to interact with protocol. The issue here is some params of lien are not emitted in the event when the lien is created or updated. As a result, users cannot find the correct state of the lien and hence are unable to interact with protocol.

Below is an example where startTime is not emitted when lien is updated in function refinanceAuction()

/* Reset the lien with the new lender and interest rate. */
_liens[lienId] = keccak256(
    abi.encode(
        Lien({
            lender: msg.sender,
            borrower: lien.borrower,
            collection: lien.collection,
            tokenId: lien.tokenId,
            amount: debt,
            startTime: block.timestamp, // @audit not emit event, hard to track
            rate: rate,
            auctionStartBlock: 0
        })
    )
);

/* Repay the initial loan. */
pool.transferFrom(msg.sender, lien.lender, debt);

emit Refinance(lienId, address(lien.collection), msg.sender, debt, lien.rate);

Proof of Concept

Another example is auctionStartBlock is not emitted in function startAuction()

/* Add auction start block to lien. */
_liens[lienId] = keccak256(
    abi.encode(
        Lien({
            lender: lien.lender,
            borrower: lien.borrower,
            collection: lien.collection,
            tokenId: lien.tokenId,
            amount: lien.amount,
            startTime: lien.startTime,
            rate: lien.rate,
            auctionStartBlock: block.number // @audit not emitted
        })
    )
);

emit StartAuction(lienId, address(lien.collection));

Advanced users can still track this param by looking onchain to find the timestamp and block number of their transactions. However, this is error-prone and not recommended.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider emitting all the data of Lien when it is updated or created.

code423n4 commented 1 year ago

Withdrawn by minhquanym