search

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

2 stars 0 forks source link

`TimeswapPair.sol#lock()` Switching between 1, 2 instead of 0, 1 is more gas efficient #150

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Core/contracts/TimeswapPair.sol#L121-L126

    modifier lock() {
        require(locked == 0, 'E211');
        locked = 1;
        _;
        locked = 0;
    }

SSTORE from 0 to 1 (or any non-zero value), the cost is 20000;

SSTORE from 1 to 2 (or any other non-zero value), the cost is 5000.

By storing the original value once again, a refund is triggered (https://eips.ethereum.org/EIPS/eip-2200).

Since refunds are capped to a percentage of the total transaction's gas, it is best to keep them low, to increase the likelihood of the full refund coming into effect.

Therefore, switching between 1, 2 instead of 0, 1 will be more gas efficient.

See: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/86bd4d73896afcb35a205456e361436701823c7a/contracts/security/ReentrancyGuard.sol#L29-L33

amateur-dev commented 2 years ago

Similar issue reported over here #87; hence closing this