Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The project has the risk of getting a Vampire Attack due to the fixed fee rate #192

Closed
code423n4 opened this issue Dec 17, 2022 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-203 edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

code423n4 commented Dec 17, 2022

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Caviar.sol#L12

Vulnerability details

Traders pay a 30bps (0.3%) fee each time they buy or sell. This fee accrues to the liquidity providers and acts as an incentive for people to deposit liquidity.

However, the competition in NFT projects is high, especially with the Vampire Attack method, the platforms take the share of the other.
The fee rate, which is the most important point of the platform for the user, is fixed, there is a risk that a similar project in the future will only increase the fee rate and take the liquidity here.

What is a Vampire Attack in Crypto?
https://medium.com/@buraktahtacioglu/looksrare-vampire-attack-on-opensea-blockchain-roadmap-29bd51753a64
https://omerkeman.medium.com/what-is-a-vampire-attack-in-crypto-fdfc5e1fc5fc

Although the examples above are Vampire Attack, they do not fully comply with our findings here, because while this is done with airdrop, it is thought that this will be done with the fee rate in our case.

Proof of Concept

1- Caviar platform works with a fixed 0.3% commission rate
2- With the hype of NFTs, competition increases and platforms increase and a competing project charges 0.1% on each trade to get a share of existing users of Caviar instead of finding new users, and many NFT AMM users, including Caviar, offer an airdrop opportunity.
3- Users with a large number of trading movements receive a share and due to the fixed fee rate, the Caviar platform cannot react to it

Recommended Mitigation Steps

Instead of a fixed fee rate, this problem can be solved in two ways;
1- Function is added so that the rate of Fee can be updated by an authorized owner
2- Core Contract can be made upgradable (A different architecture is required for this)
3- The commission rate should be fixed with a certain upper rate so that users will not have a trust problem.

Uniswap v3 solved this problem as follows;

 /// @inheritdoc IUniswapV3PoolOwnerActions
    function setFeeProtocol(uint8 feeProtocol0, uint8 feeProtocol1) external override lock onlyFactoryOwner {
        require(
            (feeProtocol0 == 0 || (feeProtocol0 >= 4 && feeProtocol0 <= 10)) &&
                (feeProtocol1 == 0 || (feeProtocol1 >= 4 && feeProtocol1 <= 10))
        );
        uint8 feeProtocolOld = slot0.feeProtocol;
        slot0.feeProtocol = feeProtocol0 + (feeProtocol1 << 4);
        emit SetFeeProtocol(feeProtocolOld % 16, feeProtocolOld >> 4, feeProtocol0, feeProtocol1);
    }

NFTX solved this issue as follows;

https://github.com/NFTX-project/nftx-protocol-v2/blob/master/contracts/solidity/NFTXVaultFactoryUpgradeable.sol#L77-L97

 function setFactoryFees(
        uint256 mintFee, 
        uint256 randomRedeemFee, 
        uint256 targetRedeemFee,
        uint256 randomSwapFee, 
        uint256 targetSwapFee
    ) public onlyOwner virtual override {
        require(mintFee <= 0.5 ether, "Cannot > 0.5 ether");
        require(randomRedeemFee <= 0.5 ether, "Cannot > 0.5 ether");
        require(targetRedeemFee <= 0.5 ether, "Cannot > 0.5 ether");
        require(randomSwapFee <= 0.5 ether, "Cannot > 0.5 ether");
        require(targetSwapFee <= 0.5 ether, "Cannot > 0.5 ether");

        factoryMintFee = uint64(mintFee);
        factoryRandomRedeemFee = uint64(randomRedeemFee);
        factoryTargetRedeemFee = uint64(targetRedeemFee);
        factoryRandomSwapFee = uint64(randomSwapFee);
        factoryTargetSwapFee = uint64(targetSwapFee);

        emit UpdateFactoryFees(mintFee, randomRedeemFee, targetRedeemFee, randomSwapFee, targetSwapFee);
    }
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 17, 2022
code423n4 added a commit that referenced this issue Dec 17, 2022
@Shungy
Copy link
Member

Shungy commented Jan 2, 2023

This reads more like informational. That would mean this submission's severity is overinflated.

@outdoteth
Copy link

Vampire attack design is out of scope

@c4-sponsor
Copy link

outdoteth marked the issue as sponsor disputed

@c4-sponsor c4-sponsor added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Jan 5, 2023
C4-Staff added a commit that referenced this issue Jan 6, 2023
@berndartmueller
Copy link
Member

Downgrading to QA (NC) as this represents a recommendation to reconsider the design of the fee mechanism of the protocol.

@c4-judge c4-judge added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax downgraded by judge Judge downgraded the risk level of this issue duplicate-203 and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 10, 2023
@c4-judge
Copy link
Contributor

Duplicate of #203

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-203 edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

6 participants