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

zhuying - The functions about permit won't work and always revert #40

Open
sherlock-admin opened this issue Mar 5, 2024 · 3 comments
Open
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 5, 2024

zhuying

high

The functions about permit won't work and always revert

Summary

The functions about permit won't work and always revert

Vulnerability Detail

JalaRouter02.sol has functions (removeLiquidityWithPermit/removeLiquidityETHWithPermit/removeLiquidityETHWithPermitSupportingFeeOnTransferTokens) about permit. These functions will call permit function in JalaPair.sol. JalaPair is inherited from JalaERC20. Although JalaERC20 is out of scope. But both JalaPair and JalaERC20 have no permit functions. So when you call removeLiquidityWithPermit/removeLiquidityETHWithPermit/removeLiquidityETHWithPermitSupportingFeeOnTransferTokens, it will always revert.

POC

Add this test function in JalaRouter02.t.sol.

function test_Permit() public {
        tokenA.approve(address(router), 1 ether);
        tokenB.approve(address(router), 1 ether);

        router.addLiquidity(
            address(tokenA), address(tokenB), 1 ether, 1 ether, 1 ether, 1 ether, address(this), block.timestamp
        );

        address pairAddress = factory.getPair(address(tokenA), address(tokenB));
        JalaPair pair = JalaPair(pairAddress);
        uint256 liquidity = pair.balanceOf(address(this));

        liquidity = (liquidity * 3) / 10;
        pair.approve(address(router), liquidity);

        vm.expectRevert();
        router.removeLiquidityWithPermit(
            address(tokenA),
            address(tokenB),
            liquidity,
            0.3 ether - 300,
            0.3 ether - 300,
            address(this),
            block.timestamp,
            true,
            1, // this value is for demonstration only
            bytes32(uint256(1)), // this value is for demonstration only
            bytes32(uint256(1)) // this value is for demonstration only
        );
    }

Impact

We can't remove liquidity by using permit.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaRouter02.sol#L150-L167
https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaRouter02.sol#L169-L185
https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaRouter02.sol#L202-L225

Tool used

manual review and foundry

Recommendation

Implement permit function in JalaERC20. Reference: https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2ERC20.sol.

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Mar 9, 2024
@sherlock-admin2 sherlock-admin2 added Sponsor Disputed The sponsor disputed this issue's validity Will Fix The sponsor confirmed this issue will be fixed Sponsor Confirmed The sponsor acknowledged this issue is valid and removed Will Fix The sponsor confirmed this issue will be fixed Sponsor Disputed The sponsor disputed this issue's validity labels Mar 11, 2024
@sherlock-admin4
Copy link

The protocol team fixed this issue in PR/commit https://github.com/jalaswap/jalaswap-dex-contract/commit/c73dda6e81268bb329ec80ef851707f3b95ff0df.

@sherlock-admin4 sherlock-admin4 changed the title Refined Porcelain Beetle - The functions about permit won't work and always revert zhuying - The functions about permit won't work and always revert Mar 18, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Mar 18, 2024
@spacegliderrrr
Copy link
Collaborator

fix looks good, permit function is now added

@sherlock-admin4
Copy link

The Lead Senior Watson signed off on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants