Using safeTransferFrom() Functions But without Getting the Approval

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L95 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L172 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L172 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L172 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L172

Vulnerability details

Impact

2 safeTransferFrom calls are Used for ERC20 Token [Line: 90, 172] using SafeTransferLib But without getting the Approval using safeApprove() function. The Calls will always fail.

3 more safeTransferFrom calls are Used for ERC721 Tokens [Line: 239, 259, 370] using inbuilt Function in ERC721. But Again without Approval, the Calls will fail.

Proof of Concept

https://ethereum.org/en/developers/tutorials/transfers-and-approval-of-erc-20-tokens-from-a-solidity-smart-contract/

Recommended Mitigation Steps

For ERC20, Use safeApprove() Function to get the Approval first to Transfer the Token. And for ERC721, use approve() or setApprovalForAll() function to get the Approval Before using transferFrom() Function.

[iFrostizz]

For the "pull" pattern, the approval usually has to be sent by the sender. If the tokens are not approved, then the the transfer is going to fail too and nothing will happen. This would be an issue if for instance another contract was moving funds from the Pair.sol contract, and in this case the Pair.sol would need some kind of approval to the contract but this is not the case currently.

[c4-judge]

berndartmueller marked the issue as unsatisfactory: Invalid