search

code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Fee-on-transfer token not supported #239

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

In joinPool the amount of token required from the sender is calculated as follow https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/facets/Basket/BasketFacet.sol#L162

           uint256 tokenAmount =
                balance(address(token)).mul(_amount.add(feeAmount)).div(
                    totalSupply
                );
            require(tokenAmount != 0, "AMOUNT_TOO_SMALL");
            token.safeTransferFrom(msg.sender, address(this), tokenAmount);

If token is a fee on transfer token, the basket would receive less than tokenAmount which is undesirable

Proof of Concept

Consider 1) 50:50 basket of $ETH and $SHIT 2) A deposit 1 $ETH and 1 $SHIT for 100 $BASKET 3) $SHIT take 10% fee on transfer 4) Pool now have 1 $ETH and 0.9 $SHIT 5) B deposit 1 $ETH and 0.9 $SHIT for 100 $BASKET 6) Pool now have 2 $ETH and 1.71 $SHIT

A and B got the same amount of $BASKET despite B pay less $SHIT

Recommended Mitigation Steps

Check before and after balance to make sure tokenAmount is received, but it is a bit tricky as how to transfer the right amount of token to account for the fee. Need to make sure user don't add fee-on-transfer token to the basket.

0xleastwood commented 2 years ago

@loki-sama I can't see anywhere to suggest that you guys intend to support these types of tokens. Can you confirm?

0xleastwood commented 2 years ago

Regardless, I don't think I can argue the issue to be high. At most it would be medium. So I'll mark it down until @loki-sama can respond.

0xleastwood commented 2 years ago

Duplicate of #220