code-423n4 / 2023-01-popcorn-findings

0 stars 0 forks source link

Upgraded Q -> 2 from #795 [1677634051278] #851

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #795 as 2 risk. The relevant finding follows:

[01] FEE-ON-TRANSFER TOKENS ARE NOT SUPPORTED This protocol currently does not support fee-on-transfer tokens. For example, for a fee-on-transfer token, calling the following Vault.deposit function with the assets input being 1e18 can transfer less than 1e18 of such token to the Vault contract since the transfer fee is deducted during the transfer; then, when this function futher calls the AdapterBase.deposit function with the assets input still being 1e18, IERC20(asset()).safeTransferFrom(caller, address(this), assets) is executed but this execution reverts because the Vault contract does not have sufficient balance of such token for transferring 1e18 again to the AdapterBase contract. To support fee-on-transfer tokens, functions like Vault.deposit function, which needs to transfer the received assets out again, can be updated to calculate the actual token amount that is received and then transfer out that received amount instead of the assets input value.

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/Vault.sol#L134-L158

function deposit(uint256 assets, address receiver)
    public
    nonReentrant
    whenNotPaused
    syncFeeCheckpoint
    returns (uint256 shares)
{
    ...
    asset.safeTransferFrom(msg.sender, address(this), assets);

    adapter.deposit(assets, address(this));
    ...
}

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L110-L122

function deposit(uint256 assets, address receiver)
    public
    virtual
    override
    returns (uint256)
{
    ...
    _deposit(_msgSender(), receiver, assets, shares);
    ...
}

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L147-L165

function _deposit(
    address caller,
    address receiver,
    uint256 assets,
    uint256 shares
) internal nonReentrant virtual override {
    IERC20(asset()).safeTransferFrom(caller, address(this), assets);
    ...
}
c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #503

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory