Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L130 https://github.com/smartcontractkit/chainlink/blob/b57617ed2249ac711db75e5bef5a0a78bf10b2aa/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.7.0/contracts/token/ERC20/utils/SafeERC20.sol#L30-L37

Vulnerability details

Impact

Because the caller(msg.sender) never gives approval for ERC20 transfers, calls to safeTransferFrom on the contract will revert with insufficient approval. This will trap from caller and unable to transfer ticketPrice to Lottery contract address. The root cause of this issue is misusing safeTransferFrom to transfer tokens directly out of the caller(msg.sender) instead of using transfer directly. So the caller(msg.sender) no need to approve ticketPrice

Proof of Concept

rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);

(https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L130)

SafeERC20.sol#

function safeTransferFrom( IERC20 token, address from, address to, uint256 value ) internal { _callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value)); }

(https://github.com/smartcontractkit/chainlink/blob/b57617ed2249ac711db75e5bef5a0a78bf10b2aa/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.7.0/contracts/token/ERC20/utils/SafeERC20.sol)

safeTransferFrom() internally only call the transferFrom() . Not clled any approve function before calling the transferFrom() function

When we see the that safeTranferFrom function only check the failures not given any approvals for any ticketPrice .Anywhere in this protocol the approve function is not called. When we call safeTransferFrom() function this assumes the caller already approved the tokens.

The transferFrom function is a standard function defined by the IERC20 interface that allows a third-party to transfer tokens from the balance of the from address to the balance of the to address.

To do so, the third-party must have been previously approved by the from address to transfer tokens on its behalf, using the approve function of the IERC20 interface.

SafeTransferFrom function defined in the IERC20 interface does not call any approve function internally. Only if the from address has previously approved the transfer by the third-party, using the approve function.

The approve function is a separate function in the IERC20 interface that allows the from address to grant approval to a third-party address to spend a specified number of tokens from its balance. third-party can then use the safetransferFrom function to transfer those approved tokens to another address

So without prior approval the safeTransferFrom() function fails to transfer ticketPrice because of lack of token approval

To achieve the intended behavior, the call to safeTransferFrom should be replaced with a call to transfer(to, amount), which does not require prior approval and will execute the transfer without failure or If need to use safeTranferFrom() must given approval before transfer the rewards tokens

Tools Used

Manual Audit

Recommended Mitigation Steps

Solutions:

1) Call transfer(to, amount) instead of safeTrasferFrom() function

2) Still need to use safeTransferFrom() function then first call safeApprove() function then proceed for transfer operations.

code-423n4 / 2023-03-wenwin-findings