Salty.IO - Audinarey's results

An Ethereum-based DEX with zero swap fees, yield-generating Automatic Arbitrage, and a native WBTC/WETH backed stablecoin.

General Information

Platform: Code4rena

Start Date: 16/01/2024

Pot Size: $80,000 USDC

Total HM: 37

Participants: 178

Period: 14 days

Judge: Picodes

Total Solo HM: 4

Id: 320

League: ETH

Salty.IO

Findings Distribution

Researcher Performance

Rank: 38/178

Findings: 3

Award: $385.27

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L70-L76 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L64-L71 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L107

Vulnerability details

Impact

NOTE: This is a different issue from the other one I submit regarding liquidation, this one is a different vulnerability although the same underlying cause.

When a borrowed position becomes deliquent during a cooldown period, the position cannot be liquidated, this will lead to bad debt in the protocol until the cooldown is over.

Vulnerability Details

Currently the cooldown period is 1 hr but it can go as high as 6 hr. When a user adds liquidity by calling depositCollateralAndIncreaseShare(...) in the CollateralAndLiquidity.sol contract, an internal call is made to

_depositLiquidityAndIncreaseShare(...) as shown below

function depositCollateralAndIncreaseShare( uint256 maxAmountWBTC, uint256 maxAmountWETH, uint256 minLiquidityReceived, uint256 deadline, bool useZapping ) external nonReentrant ensureNotExpired(deadline)  returns (uint256 addedAmountWBTC, uint256 addedAmountWETH, uint256 addedLiquidity)
		{
		// Have the user deposit the specified WBTC/WETH liquidity and increase their collateral share
		(addedAmountWBTC, addedAmountWETH, addedLiquidity) = _depositLiquidityAndIncreaseShare( wbtc, weth, maxAmountWBTC, maxAmountWETH, minLiquidityReceived, useZapping );

		...
		}

this in turn makes a call to the StakingRewards._increaseUserShare(...) function which executes with useCoolDown set to true.

function _depositLiquidityAndIncreaseShare( IERC20 tokenA, IERC20 tokenB, uint256 maxAmountA, uint256 maxAmountB, uint256 minLiquidityReceived, bool useZapping ) internal returns (uint256 addedAmountA, uint256 addedAmountB, uint256 addedLiquidity)
		{
		...

		// Increase the user's liquidity share by the amount of addedLiquidity.
		// Cooldown is specified to prevent reward hunting (ie - quickly depositing and withdrawing large amounts of liquidity to snipe rewards as they arrive)
		// _increaseUserShare confirms the pool as whitelisted as well.
		_increaseUserShare( msg.sender, poolID, addedLiquidity, true );

		...

		emit LiquidityDeposited(msg.sender, address(tokenA), address(tokenB), addedAmountA, addedAmountB, addedLiquidity);
		}

When the user makes a borrow immediately after increasing their shares and the collateralisation decreases, all calls to CollateralAndLiquidity.liquidateUser(...) will revert.

Coded POC

Add the following test case to the CollateralAndLiquidity.t.sol file and run COVERAGE="yes" NETWORK="sep" forge test --mt testLiquidatePositionDuringCooldownWillRevert -vv --rpc-url https://rpc.ankr.com/eth_sepolia

function testLiquidatePositionDuringCooldownWillRevert() public {

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 0 );

		assertEq(wbtc.balanceOf(address(usds)), 0, "USDS contract should start with zero WBTC");
		assertEq(weth.balanceOf(address(usds)), 0, "USDS contract should start with zero WETH");
		assertEq(usds.balanceOf(alice), 0, "Alice should start with zero USDS");

		// Total needs to be worth at least $2500
		uint256 depositedWBTC = ( 1000 ether *10**8) / priceAggregator.getPriceBTC();
		uint256 depositedWETH = ( 1000 ether *10**18) / priceAggregator.getPriceETH();

		(uint256 reserveWBTC, uint256 reserveWETH) = pools.getPoolReserves(wbtc, weth);
		assertEq( reserveWBTC, 0, "reserveWBTC doesn't start as zero" );
		assertEq( reserveWETH, 0, "reserveWETH doesn't start as zero" );

		vm.startPrank(alice);
		(,, uint256 aliceInitialAddedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("aliceInitialAddedLiquidityIncrement", aliceInitialAddedLiquidityIncrement );
		uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        // emit log_named_uint("initial maxUSDS", maxUSDS );
		assertEq( maxUSDS, 0, "Alice doesn't have enough collateral to borrow USDS" );

		// Deposit again
		vm.warp( block.timestamp + 1 hours );
		(,, uint256 addedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("alice Increment Liquidity", addedLiquidityIncrement );

		// Account for both deposits
		depositedWBTC = depositedWBTC * 2;
		depositedWETH = depositedWETH * 2;
		vm.stopPrank();

        emit log_named_uint("alice total Added Liquidity", addedLiquidityIncrement + aliceInitialAddedLiquidityIncrement );

		// Deposit extra so alice can withdraw all her liquidity without having to worry about the DUST reserve limit
		vm.prank(DEPLOYER);
		(,, uint256 addedLiquidityIncrementDeployer) = collateralAndLiquidity.depositCollateralAndIncreaseShare( 1 * 10**8, 1 ether, 0, block.timestamp, false );
        emit log_named_uint("deployer's Increment Liquidity", addedLiquidityIncrementDeployer );
        
		// vm.warp( block.timestamp + 1 hours );

		vm.prank(alice);
		maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);

		vm.startPrank(alice);

        {
		maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        // emit log_named_uint("maxUSDS", maxUSDS );

		collateralAndLiquidity.borrowUSDS( maxUSDS );
		vm.stopPrank();

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 1 );

		uint256 maxWithdrawable = collateralAndLiquidity.maxWithdrawableCollateral(alice);
		assertEq( maxWithdrawable, 0, "Alice shouldn't be able to withdraw any collateral" );

		
		uint256 aliceCollateralValue = collateralAndLiquidity.userCollateralValueInUSD(alice);

		uint256 aliceBorrowedUSDS = usds.balanceOf(alice);
		assertEq( collateralAndLiquidity.usdsBorrowedByUsers(alice), aliceBorrowedUSDS, "Alice amount USDS borrowed not what she has" );

		// Borrowed USDS should be about 50% of the aliceCollateralValue
		assertTrue( aliceBorrowedUSDS > ( aliceCollateralValue * 499 / 1000 ), "Alice did not borrow sufficient USDS" );
		assertTrue( aliceBorrowedUSDS < ( aliceCollateralValue * 501 / 1000 ), "Alice did not borrow sufficient USDS" );
		}

		vm.warp( block.timestamp + 1001);

        // @audit Within an hour of borrowing the price of collateral crashes
        // @audit this can go up to six hours with same results
        _crashCollateralPrice();

		vm.prank(bob);
        vm.expectRevert("Must wait for the cooldown to expire");
		collateralAndLiquidity.liquidateUser(alice);

    }

SUGGESTION

Modify the liquidateUser(...) function not to use the cooldown as shown below

Remove

_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );

and Add

_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );

function liquidateUser( address wallet ) external nonReentrant
		{
		...

		
		// @audit SUGGESTION: _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );
-		_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );
+		_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );

		...
		}

Assessed type

Timing

#0 - c4-judge

2024-02-01T10:45:43Z

Picodes marked the issue as duplicate of #312

#1 - c4-judge

2024-02-17T18:49:43Z

Picodes marked the issue as satisfactory

#2 - c4-judge

2024-02-21T16:12:39Z

Picodes removed the grade

#3 - c4-judge

2024-02-21T16:21:33Z

Picodes marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L107 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L154 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L70

Vulnerability details

Impact

NOTE: This is a different issue from the other one I submit regarding liquidation, this one is a different attack path although the same underlying cause.

The borrower can front run his liquidatable position to prevent liquidation. This will lead to the borrower buying time to repay, recollateralize or delay until price returns to levels where the position is collateralised again

POC Summary

  • When a borrower notices his position is insolvent
  • he adds collateral with at most DUST + 1 wei (101 wei) amount to activate cooldown
  • Liquidator calls liquidateUser(...) but it reverts because cooldown is now active
  • On the Negative side a borrower can use this to avoid being liquidated or can use this to buy time until he can repay the loan cooldown can range from 15mins to 6hrs and it is worse yet for higher cooldown periods

Coded POC

Add the following test case to the CollateralAndLiquidity.t.sol file and run COVERAGE="yes" NETWORK="sep" forge test --mt testLiquidatePositionBorrowerFrontrunsWithCooldown -vv --rpc-url https://rpc.ankr.com/eth_sepolia

function testLiquidatePositionBorrowerFrontrunsWithCooldown() public {

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 0 );

		assertEq(wbtc.balanceOf(address(usds)), 0, "USDS contract should start with zero WBTC");
		assertEq(weth.balanceOf(address(usds)), 0, "USDS contract should start with zero WETH");
		assertEq(usds.balanceOf(alice), 0, "Alice should start with zero USDS");

		// Total needs to be worth at least $2500
		uint256 depositedWBTC = ( 1000 ether *10**8) / priceAggregator.getPriceBTC();
		uint256 depositedWETH = ( 1000 ether *10**18) / priceAggregator.getPriceETH();

        emit log_named_uint("depositedWBTC", depositedWBTC );
        emit log_named_uint("depositedWETH", depositedWETH );

		(uint256 reserveWBTC, uint256 reserveWETH) = pools.getPoolReserves(wbtc, weth);
		assertEq( reserveWBTC, 0, "reserveWBTC doesn't start as zero" );
		assertEq( reserveWETH, 0, "reserveWETH doesn't start as zero" );

		vm.startPrank(alice);
		(,, uint256 aliceInitialAddedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("aliceInitialAddedLiquidityIncrement", aliceInitialAddedLiquidityIncrement );
		uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        // emit log_named_uint("initial maxUSDS", maxUSDS );
		assertEq( maxUSDS, 0, "Alice doesn't have enough collateral to borrow USDS" );

		// Deposit again
		vm.warp( block.timestamp + 1 hours );
		(,, uint256 addedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("alice Increment Liquidity", addedLiquidityIncrement );

		// Account for both deposits
		depositedWBTC = depositedWBTC * 2;
		depositedWETH = depositedWETH * 2;
		vm.stopPrank();

        emit log_named_uint("alice total Added BEFORE borrow Liquidity", addedLiquidityIncrement + aliceInitialAddedLiquidityIncrement );

		// Deposit extra so alice can withdraw all liquidity without having to worry about the DUST reserve limit
		vm.prank(DEPLOYER);
		(,, uint256 addedLiquidityIncrementDeployer) = collateralAndLiquidity.depositCollateralAndIncreaseShare( 1 * 10**8, 1 ether, 0, block.timestamp, false );
        emit log_named_uint("deployer's Increment Liquidity", addedLiquidityIncrementDeployer );
        
		// vm.warp( block.timestamp + 1 hours );

		vm.prank(alice);
		maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);

		vm.startPrank(alice);

        {
		maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        // emit log_named_uint("maxUSDS", maxUSDS );

		collateralAndLiquidity.borrowUSDS( maxUSDS );
		vm.stopPrank();

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 1 );

		uint256 maxWithdrawable = collateralAndLiquidity.maxWithdrawableCollateral(alice);
		assertEq( maxWithdrawable, 0, "Alice shouldn't be able to withdraw any collateral" );

		
		uint256 aliceCollateralValue = collateralAndLiquidity.userCollateralValueInUSD(alice);

		uint256 aliceBorrowedUSDS = usds.balanceOf(alice);
		assertEq( collateralAndLiquidity.usdsBorrowedByUsers(alice), aliceBorrowedUSDS, "Alice amount USDS borrowed not what she has" );

		// Borrowed USDS should be about 50% of the aliceCollateralValue
		assertTrue( aliceBorrowedUSDS > ( aliceCollateralValue * 499 / 1000 ), "Alice did not borrow sufficient USDS" );
		assertTrue( aliceBorrowedUSDS < ( aliceCollateralValue * 501 / 1000 ), "Alice did not borrow sufficient USDS" );
		}

        // @audit 1 day passes before the price tanks
		vm.warp( block.timestamp + 1 days);

        // @audit position becomes under collateralized
        _crashCollateralPrice();

        // @audit add little of liquidity
        vm.startPrank(alice);
		(,, uint256 aliceInitialAddedLiquidityToFrontrun) = collateralAndLiquidity.depositCollateralAndIncreaseShare( 101 wei, 101 wei, 0, block.timestamp, false );
        emit log_named_uint("aliceInitial NEW TOTAL AddedLiquidityIncrement", aliceInitialAddedLiquidityToFrontrun + addedLiquidityIncrement + aliceInitialAddedLiquidityIncrement );

        // @audit borrower still cant borrow because he is not eligible despite added liquidity
		uint256 maxUSDSAfterFrontrun = collateralAndLiquidity.maxBorrowableUSDS(alice);
        emit log_named_uint("FINAL maxUSDS", maxUSDSAfterFrontrun );
		assertEq( maxUSDSAfterFrontrun, 0, "Alice doesn't have enough collateral to borrow USDS" );
        vm.stopPrank();

        // @audit 10 seconds after
        vm.warp( block.timestamp + 10);

        // @audit Bob's call to liquidate reverts
		vm.prank(bob);
        vm.expectRevert("Must wait for the cooldown to expire");
		collateralAndLiquidity.liquidateUser(alice);

        // @audit user can kep spamming until price returns to levels where the position becomes collateralised again
        // OR 
        // @audit user can use this to buy time to repay or recollateralise his position

    }

SUGGESTION

Modify the liquidateUser(...) function not to use the cooldown as shown below

Remove

_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );

and Add

_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );

function liquidateUser( address wallet ) external nonReentrant
		{
		require( wallet != msg.sender, "Cannot liquidate self" );

		...

		
		// @audit SUGGESTION: _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );
-		_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );
+		_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );
		...

		emit Liquidation(msg.sender, wallet, reclaimedWBTC, reclaimedWETH, originallyBorrowedUSDS);
		}

.

Assessed type

Timing

#0 - c4-judge

2024-01-31T22:38:02Z

Picodes marked the issue as duplicate of #891

#1 - c4-judge

2024-02-21T16:12:36Z

Picodes marked the issue as satisfactory

Findings Information

🌟 Selected for report: Banditx0x

Also found by: Audinarey, Giorgio, t0x1c

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sponsor disputed
duplicate-279

Awards

372.7994 USDC - $372.80

External Links

Lines of code

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

Vulnerability details

Impact

Malicious user can spam the protocol with unliquidatable positions leaving the CollateralAndLiquidity.sol pool with dead positions.

POC Summary

This vulnerability is borne form the fact that the user is allowed to repay any amount they like

  • Borrower takes up loan
  • repays up between 90% to 95% of the loan
  • borrower may decide to withdraws max collateral withdrawable after repayment
  • price of collateral drops and position becomes undercollateralised
  • liquidator calling liquidateUser(...) will revert for the outstanding balance and as such position is left underwater or the borrower decides to repay whenever he deems fit spaming the protocol with dead positions
  • Borrower can withdraw most of their liquidity.
  • There is a higher propensity for this attack when the collateralisation ratio is still close to 110%

Coded POC

Add the following test case to the CollateralAndLiquidity.t.sol file and run COVERAGE="yes" NETWORK="sep" forge test --mt testLiquidateIsNotProfitableForSpamPositions -vv --rpc-url https://rpc.ankr.com/eth_sepolia

function testLiquidateIsNotProfitableForSpamPositions() public {

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 0 );

		assertEq(wbtc.balanceOf(address(usds)), 0, "USDS contract should start with zero WBTC");
		assertEq(weth.balanceOf(address(usds)), 0, "USDS contract should start with zero WETH");
		assertEq(usds.balanceOf(alice), 0, "Alice should start with zero USDS");

		// Total needs to be worth at least $2500
		uint256 depositedWBTC = ( 1000 ether *10**8) / priceAggregator.getPriceBTC();
		uint256 depositedWETH = ( 1000 ether *10**18) / priceAggregator.getPriceETH();

        emit log_named_uint("depositedWBTC", depositedWBTC );
        emit log_named_uint("depositedWETH", depositedWETH );

		(uint256 reserveWBTC, uint256 reserveWETH) = pools.getPoolReserves(wbtc, weth);
		assertEq( reserveWBTC, 0, "reserveWBTC doesn't start as zero" );
		assertEq( reserveWETH, 0, "reserveWETH doesn't start as zero" );

		vm.startPrank(alice);
		(,, uint256 aliceInitialAddedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("aliceInitialAddedLiquidityIncrement", aliceInitialAddedLiquidityIncrement );
		uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        // emit log_named_uint("initial maxUSDS", maxUSDS );
		assertEq( maxUSDS, 0, "Alice doesn't have enough collateral to borrow USDS" );

		// Deposit again
		vm.warp( block.timestamp + 1 hours );
		(,, uint256 addedLiquidityIncrement) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
        emit log_named_uint("alice Increment Liquidity", addedLiquidityIncrement );

		// Account for both deposits
		depositedWBTC = depositedWBTC * 2;
		depositedWETH = depositedWETH * 2;
		vm.stopPrank();

        emit log_named_uint("alice total Added BEFORE borrow Liquidity", addedLiquidityIncrement + aliceInitialAddedLiquidityIncrement );

		// Deposit extra so alice can withdraw all liquidity without having to worry about the DUST reserve limit
		vm.prank(DEPLOYER);
		(,, uint256 addedLiquidityIncrementDeployer) = collateralAndLiquidity.depositCollateralAndIncreaseShare( 1 * 10**8, 1 ether, 0, block.timestamp, false );
        emit log_named_uint("deployer's Increment Liquidity", addedLiquidityIncrementDeployer );
        
		// vm.warp( block.timestamp + 1 hours );

        // @audit alice borrows USDS
		vm.prank(alice);
		maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);

		vm.startPrank(alice);
        maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);

         // @audit calculate amount to repay 
        uint256 amountUSDSToRepay = maxUSDS * 90 / 100; 

        {

		collateralAndLiquidity.borrowUSDS( maxUSDS );
		vm.stopPrank();

		assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 1 );

		uint256 maxWithdrawable = collateralAndLiquidity.maxWithdrawableCollateral(alice);
		assertEq( maxWithdrawable, 0, "Alice shouldn't be able to withdraw any collateral" );

		
		uint256 aliceCollateralValue = collateralAndLiquidity.userCollateralValueInUSD(alice);

		uint256 aliceBorrowedUSDS = usds.balanceOf(alice);
		assertEq( collateralAndLiquidity.usdsBorrowedByUsers(alice), aliceBorrowedUSDS, "Alice amount USDS borrowed not what she has" );

		// Borrowed USDS should be about 50% of the aliceCollateralValue
		assertTrue( aliceBorrowedUSDS > ( aliceCollateralValue * 499 / 1000 ), "Alice did not borrow sufficient USDS" );
		assertTrue( aliceBorrowedUSDS < ( aliceCollateralValue * 501 / 1000 ), "Alice did not borrow sufficient USDS" );
		}

        // @audit 1 day passes before the price tanks
		vm.warp( block.timestamp + 1 days);

        // @audit repay 
        vm.startPrank(alice);
        collateralAndLiquidity.repayUSDS(amountUSDSToRepay);
        vm.stopPrank();

        uint256 maxUSDSAfterSpamRepayment = collateralAndLiquidity.maxBorrowableUSDS(alice);

        // @audit ailce maxWithdrawableCollateral
        uint256 withdrawableCollateral = collateralAndLiquidity.maxWithdrawableCollateral(alice);
        emit log_named_uint("Alice withdrawable collateral after spam payment", withdrawableCollateral );

        assertTrue(collateralAndLiquidity.usdsBorrowedByUsers(alice) != 0, "Alice has paid complete");
        emit log_named_uint("Alice outstanding Borrowable Balance", maxUSDSAfterSpamRepayment );
        emit log_named_uint("Alice collateral value", collateralAndLiquidity.userCollateralValueInUSD(alice) );

        // @audit position becomes under collateralized
        _crashCollateralPrice();

        // @audit Bob's call to liquidate reverts
		vm.prank(bob);
        vm.expectRevert("User cannot be liquidated");
		collateralAndLiquidity.liquidateUser(alice);

					// @audit user can withdraw most of their collateral
    }

SUGGESTION

  • Ensure users cannot have partially repay below 40 to 50% of the borrowed amount
  • create a mapping

mapping(address=>uint256) public minOutstandingUsersBalance;

  • Modify the CollateralAndLiquidity.borrowUSDS(...) function as shown below
function borrowUSDS( uint256 amountBorrowed ) external nonReentrant
		{
		...

		// walet has access
		// user has more than 1 wei of shares
		// amountBorrowed <= maxBorrowableUSDS(msg.sender)

		// Increase the borrowed amount for the user
		usdsBorrowedByUsers[msg.sender] += amountBorrowed;
+		minOutstandingUsersBalance[msg.sender] += usdsBorrowedByUsers[msg.sender] * 40 / 100;

		...

		emit BorrowedUSDS(msg.sender, amountBorrowed);
		}

such that minOutstandingUsersBalance should be calculated during borrowing

  • Modify the CollateralAndLiquidity.repayUSDS(…) function as shown below
function repayUSDS( uint256 amountRepaid ) external nonReentrant
		{
			// @audit 1) does not check if user should be liquidated before granting approval for repayment
			// @audit 2) user can borrow more than collateral
		require( userShareForPool( msg.sender, collateralPoolID ) > 0, "User does not have any collateral" );
		require( amountRepaid <= usdsBorrowedByUsers[msg.sender], "Cannot repay more than the borrowed amount" );
		require( amountRepaid > 0, "Cannot repay zero amount" );

		// Decrease the borrowed amount for the user
		usdsBorrowedByUsers[msg.sender] -= amountRepaid;
+		require(usdsBorrowedByUsers[msg.sender] >= minOutstandingUsersBalance[msg.sender], "Partial Repayment threshold violated")

			// @audit SUGGESTION: require(usdsBorrowedByUsers[msg.sender] >= MIN_BORROW_AMOUNT, "Partial Repayment threshold violated")

		// Have the user send the USDS to the USDS contract so that it can later be burned (on USDS.performUpkeep)
		usds.safeTransferFrom(msg.sender, address(usds), amountRepaid)
		// Have USDS remember that the USDS should be burned
		liquidizer.incrementBurnableUSDS( amountRepaid );

		// Check if the user no longer has any borrowed USDS
		if ( usdsBorrowedByUsers[msg.sender] == 0 )
			_walletsWithBorrowedUSDS.remove(msg.sender);

		emit RepaidUSDS(msg.sender, amountRepaid);
		}

Assessed type

Other

#0 - c4-judge

2024-02-02T09:26:34Z

Picodes marked the issue as primary issue

#1 - c4-sponsor

2024-02-12T23:49:45Z

othernet-global (sponsor) disputed

#2 - othernet-global

2024-02-12T23:51:47Z

Liquidating underwater positions does not revert.

The default collateral position to borrow USDS is $2500 (adjustable by the DAO). Spamming multiple borrows with multiple accounts requires this minimum amount deposited to each account.

#3 - c4-judge

2024-02-14T07:10:17Z

Picodes marked the issue as satisfactory

#4 - c4-judge

2024-02-14T07:13:33Z

Picodes marked issue #279 as primary and marked this issue as a duplicate of 279

#5 - c4-judge

2024-02-14T07:49:11Z

Picodes changed the severity to 2 (Med Risk)

[L-01] Parameter Ballot can be proposed with invalid parameterType

If Proposals.proposeParameterBallot(...) is called with an invalid parameterType the function does not revert. I am reporting this as a LOW severity because it does not break the protocols functionality as an invalid parameterType does not do anything.

Suggestion

Modify the Proposals.proposeParameterBallot(...) as shown below

function proposeParameterBallot( uint256 parameterType, string calldata description ) external nonReentrant returns (uint256 ballotID) { + require(parameterType < 26, "Invalid ParameterType"); string memory ballotName = string.concat("parameter:", Strings.toString(parameterType) ); return _possiblyCreateProposal( ballotName, BallotType.PARAMETER, address(0), parameterType, "", description ); }

[L-02] SendSALT can be proposed with zero amount

If Proposals.proposeSendSALT(...) is called with zero as the amount of SALT to send, the function does not revert.

Although no SALT is sent even when the proposal passes but the caller still spends gas fee when the call is made.

Suggestion

  • Modify the Proposals.proposeSendSALT(...) to revert when the SALT balance is less than 100 wei (to avoid rounding errors)
function proposeSendSALT( address wallet, uint256 amount, string calldata description ) external nonReentrant returns (uint256 ballotID)
		{
		...
		uint256 balance = exchangeConfig.salt().balanceOf( address(exchangeConfig.dao()) );
		require(balance > 100 wei, "Insufficient SALT balance");
		uint256 maxSendable = balance * 5 / 100;

		...
		}

#0 - c4-judge

2024-02-03T13:13:06Z

Picodes marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter