0x52 - transferFrom uses allowance even if spender == from

[github-actions[bot]]

0x52

medium

transferFrom uses allowance even if spender == from

Summary

Pool#transferFrom attempts to use allowance even when spender = from. This breaks compatibility with a large number of protocol who opt to use the transferFrom method all the time (pull only) instead of using both transfer and transferFrom (push and pull). The ERC20 standard only does an allowance check when spender != from. The result of this difference will likely result in tokens becoming irreversibly stranded across different protocols.

Vulnerability Detail

https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Pool.sol#L284-L293

The trasnferFrom method shown above always uses allowance even if spender = from.

Impact

Token won't be compatible with some protocols and will end up stranded

Code Snippet

https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Pool.sol#L284-L293

Tool used

Solidity YouTube Tutorial

Recommendation

Only use allowance when spender != from:

    require(to != address(0), "Pool: to cannot be address 0");
+   if (from != msg.sender) {
+       allowance[from][msg.sender] -= amount;
+   }
    balanceOf[from] -= amount;

[0xMoaz]

Fixed

[0xMoaz]

https://github.com/Surge-fi/surge-protocol-v1/commit/08422f62cc5bc8b5a8f53bd14fadece66814e9de

[IAm0x52]

Fix looks good. Allowance check is bypassed if msg.sender == from