sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

HexHackers - swapper isn't approved to transfer `amountToBeneficiary` in SwapperImpl.sol #83

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

HexHackers

medium

swapper isn't approved to transfer amountToBeneficiary in SwapperImpl.sol

Summary

swapper isn't approved to transfer amountToBeneficiary

Vulnerability Detail

when tokenToBeneficiary is an ERC20 token, the swapper needs to be approved to transfer amountToBeneficiary But here approve isn't done first before the contract attempts to carryout the transfer although safeTransferFrom is used.

        } else {
            tokenToBeneficiary_.safeTransferFrom(msg.sender, _beneficiary, amountToBeneficiary_);

Impact

The msg.sender doesn't approve the contract to transfer amounToBeneficiary therefore the transfer will fail.

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L272-L274

Tool used

Manual Review

Recommendation

If tokenToBeneficiary is an ERC20, you must use approve Swapper to transfer amountToBeneficiary.