Submission Details
Severity:
Use `safeTransfer()` instead of `transfer()` and `safeTransferFrom()` instead of `transferFrom()`
Vulnerability Details
transfer() might return false instead of reverting, in this case, ignoring return value leads to considering it successful.
Context: https://github.com/Cyfrin/2024-01-Morpheus/blob/main/contracts/mock/GatewayRouterMock.sol#L15
function outboundTransfer(
address _token,
address _to,
uint256 _amount,
uint256 _maxGas,
uint256 _gasPriceBid,
bytes calldata _data
) external payable returns (bytes memory) {
@> IERC20(_token).transferFrom(msg.sender, _to, _amount);
return abi.encode(_token, _to, _amount, _maxGas, _gasPriceBid, _data);
}
function getGateway(address) external view returns (address) {
return address(this);
}
Similarly, transferFrom() also don't revert but returning false.
Context: https://github.com/Cyfrin/2024-01-Morpheus/blob/main/contracts/mock/SwapRouterMock.sol#L9
function exactInputSingle(ISwapRouter.ExactInputSingleParams calldata params_) external returns (uint256) {
IERC20(params_.tokenIn).transferFrom(msg.sender, address(this), params_.amountIn);
IERC20(params_.tokenOut).transfer(params_.recipient, params_.amountIn);
return params_.amountIn;
}
Impact
Tokens that don't actually perform the transfer and return false are still counted as a correct transfer.
Tools Used
Manual Review
Recommendations
Use safeTransfer() from Openzeppelin instead of transfer().
Updates
Prize pool breakdown
Total prize
22,500 USDC
nSLOC
1,05421 USDC / LOC
20,000 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.