code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L172 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L325

Vulnerability details

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

  - The claimer smart contract does not implement a payable function.
  - The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
  - The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.
  **Additionally**, using higher than 2300 gas might be mandatory for some multisig wallets.

Affected Functions

Migration.sol#leave(address _vault, uint256 _proposalId) external Lines: https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L141-L173 Behave: User shall not be allowed to leaves a proposed migration after his contribution amount payed using Migration.sol#join(...).
Migration.sol#withdrawContribution(address _vault, uint256 _proposalId)external Lines: https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L292-L326 Behave: Here also the user shall not be able to withdraw and retrieves ether deposited from an unsuccessful migration proposal.

Proof of Concept

src/modules/Migration.sol:
  ...
  171          // Withdraws ether from contract back to caller
  172:         payable(msg.sender).transfer(ethAmount); // @audit use call
  173      }
  ...
  324          // Withdraws ether from contract back to caller
  325:         payable(msg.sender).transfer(userEth); // @audit use call
  326      }
  ...

Recommended Mitigation Steps

I recommend using

(bool success,) = payable(msg.sender).call{value: ethAmount}("");
if (!success) revert <Error>();

or take advantage of src/utils/SafeSend.sol contract and use _sendEthOrWeth(to, amount) where it should preform a safe gas usage upon transferring ether. https://github.com/code-423n4/2022-07-fractional/blob/main/src/utils/SafeSend.sol

stevennevins commented 2 years ago

Duplicate of #325

HardlyDifficult commented 2 years ago

Duping to #504

code-423n4 / 2022-07-fractional-findings