Avoid Unnecessary Expensive SSTORE Calls In Vether.sol By Checking If _fee Is Non-Zero
SSTORE calls (writes to storage) are very expensive, especially for cold-storage slots (those that have not yet been accessed this transaction).
We know that the SSTORE call to totalFees will be a cold storage call, since this is the only place in the whole contract that totalFees is used.
Vether.sol makes two SSTORE calls in _transfer that are unnecessary when _fee is zero.
It will be common for _fee to be zero, since Vether.sol implements an "excluded addresses" list (mapAddress_Excluded), where _fee is zero when either the sender or the recipient is on the excludedAddresses list.
Currently, anyone can add themselves to the excludedAddresses list, but that is probably a mistake.
Nevertheless, since it will probably at least include Uniswap, we should add a check for whether _fee is zero.
Proof of Concept
When _fee is zero, Vether._transfer() nevertheless makes these two unnecessary SSTORE calls:
0xBrian
commented
3 years ago
Handle
jvaqa
Vulnerability details
Impact
Avoid Unnecessary Expensive SSTORE Calls In Vether.sol By Checking If _fee Is Non-Zero
SSTORE calls (writes to storage) are very expensive, especially for cold-storage slots (those that have not yet been accessed this transaction). We know that the SSTORE call to totalFees will be a cold storage call, since this is the only place in the whole contract that totalFees is used. Vether.sol makes two SSTORE calls in _transfer that are unnecessary when _fee is zero. It will be common for _fee to be zero, since Vether.sol implements an "excluded addresses" list (mapAddress_Excluded), where _fee is zero when either the sender or the recipient is on the excludedAddresses list. Currently, anyone can add themselves to the excludedAddresses list, but that is probably a mistake. Nevertheless, since it will probably at least include Uniswap, we should add a check for whether _fee is zero.
Proof of Concept
When _fee is zero, Vether._transfer() nevertheless makes these two unnecessary SSTORE calls:
_balances[address(this)] += _fee; totalFees += _fee;
Recommended Mitigation Steps
Change this:
_balances[address(this)] += _fee; totalFees += _fee;
To this:
if(_fee > 0){ _balances[address(this)] += _fee; totalFees += _fee; }