code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

cause an attacker to get a better deal on intrest rate on borrwoing and lending or cause txs' to revert #196

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashBase.sol#L83-L90

Vulnerability details

Impact

if the variable maturity in getMaturity function  is a big number  that gets put into  uint40 and since its to big of a number  it will revert because solidity ^0.8.0  dosn't allow  overflow and on deployment hasMatured can be true because  it very  close to the original block.timstamp even though it really has matured , because after each block  timestamp becomes bigger.

impact it can effect gas and even logic because you can make it mature to quickly and get better  intreast rate out of cdai instead of fdai or if its negtive intreast rate then if you can make tx after deployment right way deposit  dai and borrow for short time then quickly loan that get your money back and since hasmutred is true ( even though it shouldnt be then take the intrest into cdai it can make the protocl or users loose money.

function getMaturity() public view override returns (uint40 maturity) {
    (/* */, maturity, /* */) = EncodeDecode.decodeERC1155Id(_fCashId);
}

/// @notice True if the fCash has matured, assets mature exactly on the block time
function hasMatured() public view override returns (bool) {
    return getMaturity() <= block.timestamp;
}

Tools Used

manual analysis

Recommended Mitigation Steps

you can some kind of check for delay after deployment or you can make maturity uint256 so it can handle a bigger number.

jeffywu commented 2 years ago

I don't understand the report. uint40 is sufficient size to handle timestamps well into the foreseeable future.