code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

`XDEFIDistribution.sol#_toInt256Safe()` Validation of input value can be done earlier to save gas #147

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Check input value earlier can avoid unnecessary code execution when the check failed.

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L283-L286

function _toInt256Safe(uint256 x_) internal pure returns (int256 y_) {
    y_ = int256(x_);
    assert(y_ >= int256(0));
}

See: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4961a51cc736c7d4aa9bd2e11e4cbbaff73efee9/contracts/utils/math/SafeCast.sol#L238

Recommendation

Change to:

function _toInt256Safe(uint256 value) internal pure returns (int256) {
    // Note: Unsafe cast below is okay because `type(int256).max` is guaranteed to be positive
    require(value <= uint256(type(int256).max));
    return int256(value);
}
deluca-mike commented 2 years ago

Unfortunately, this results in more gas for the happy path and more gas for the deploy, so it's not really a valid trade-off.