code423n4 commented 2 years ago

Handle

xxxxx

Vulnerability details

Impact

Gas and smart contract size reduction by removing the unnecessary check.

Proof of Concept

In the contract "Alchemist.sol", the constants MINIMUM_COLLATERALIZATION_LIMIT and MAXIMUM_COLLATERALIZATION_LIMIT can be uint64. Moreover, the parameter of the function setCollateralizationLimit should be changed from uint256 to uint64. In that case, when calling the function setCollateralizationLimit, it is not necessary to check whether the _limit is smaller than MAXIMUM_COLLATERALIZATION_LIMIT or not, because it is checked inherently to be smaller than (2**64) due to parameter type which is changed to uint64. Thus, the codes in line 313 to 316 can be removed.

Tools Used

Recommended Mitigation Steps

Remix solidity 0.6.12 My recommendation is as follows:

uint64 public constant MAXIMUM_COLLATERALIZATION_LIMIT = 4000000000000000000; uint64 public constant MINIMUM_COLLATERALIZATION_LIMIT = 1000000000000000000;

function setCollateralizationLimit(uint64 _limit) external onlyGov { require( _limit >= MINIMUM_COLLATERALIZATION_LIMIT, 'Alchemist: collateralization limit below minimum.' );

    _ctx.collateralizationLimit = FixedPointMath.FixedDecimal(_limit);

    emit CollateralizationLimitUpdated(_limit);
}

Xuefeng-Zhu commented 2 years ago

do not seem to be right

0xleastwood commented 2 years ago

Sponsor finding is not valid so will mark as invalid.

code-423n4 / 2021-11-yaxis-findings