User can approve all tokens and not just that are required for transfer

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/tokens/ERC1155Minimal.sol#L77

Vulnerability details

Impact

setApprovalForAll function allows a user to approve all token without checking whether they are listed on Uniswap or not. This can lead to loss of fund for user if a token is swapped for another token that is not listed on uniswap.

Proof of Concept

As per the specification provided by the protocol

- Transfers of ERC1155 SFPM tokens are very limited by design. It is expected that certain accounts will be unable to transfer or receive tokens.
Some tokens may not be transferable at all.

- Any compliant ERC20 token that is part of a Uniswap V3 pool and is not a fee-on-transfer token is supported, including ERC-777 tokens.

Only specific token will be used to carried out interaction but is ot followed in the setApprovalForAll function.

setApprovalForAll function does not have any check for specific token and amount which can lead to unexpected behaviour like misuse of token or loss to the user.

 function setApprovalForAll(address operator, bool approved) public {
 isApprovedForAll[msg.sender][operator] = approved;

 emit ApprovalForAll(msg.sender, operator, approved);
 }

Tools Used

Manual Review

Recommended Mitigation Steps

The recommendation is made to approve only specific token and value to avoid any misue.

-   function setApprovalForAll(address operator, bool approved) public {
+   function setApprovalForAll(address operator, bool approved, uint256 amount, address tokenAddress) public {

 // Use the ERC-20 approve function to allow spender to spend tokens on behalf of the caller
+   IERC20(tokenAddress).approve(operator, amount);
 isApprovedForAll[msg.sender][operator] = approved;

 emit ApprovalForAll(msg.sender, operator, approved);
 }

Assessed type

Token-Transfer

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

This is the design of ERC1155