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

0 stars 0 forks source link

better placement of require check #127

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

rfa

Vulnerability details

Impact

In case if user inputing amount_ = 0, it will save execution gas cost

Proof of Concept

in XDEFIDistribution (line 255), if we replace:

require(amount != uint256(0) && amount <= MAX_TOTAL_XDEFI_SUPPLY, "INVALID_AMOUNT");

to

function relock(uint256 tokenId, uint256 lockAmount, uint256 duration, address destination) external noReenter returns (uint256 amountUnlocked, uint256 newTokenId) { // PLACE REQURE CHECK HERE (line 111) // Handle the unlock and get the amount of XDEFI eligible to withdraw. amountUnlocked_ = unlock(msg.sender, tokenId);

it can prevent relock() function to executed and save user's gas

Tools Used

Recommended Mitigation Steps

as i previously reported, just place the require check before relock() (and also locked()) function executed

deluca-mike commented 2 years ago

We want to reduce gas costs for happy path, not sad paths. Wallets will already usually prevent users from singing/broadcasting transactions that will revert.

In any case, due to the move to if-reverts and custom error messages, and the removal of the MAX_TOTAL_XDEFI_SUPPLY variable, and it's check, this is being changed to a check on the resulting computed units in )lock:

if (units == uint256(0)) revert LockResultsInNoUnits();