[WP-M8] `totalUnderlyingMinusSponsored()` may revert on underflow and malfunction the contract

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L290-L293

function totalUnderlyingMinusSponsored() public view returns (uint256) {
    // TODO no invested amount yet
    return totalUnderlying() - totalSponsored;
}

As a function that many other functions depended on, totalUnderlyingMinusSponsored() can revert on underflow when sponsorAmount > totalUnderlying() which is possible and has been considered elsewhere in this contract:

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L390-L392

if (_force && sponsorAmount > totalUnderlying()) {
    sponsorToTransfer = totalUnderlying();
}

PoC

• Underlying token = USDT
• Swap Fee = 0.04%

1. Sponsor call sponsor() and send 10,000 USDT

• totalSponsored = 10,000

2. NonUSTStrategy.sol#doHardWork() swapped USDT for UST

• pendingDeposits = 9,996
• totalUnderlying() = 9,996

3. Alice tries to call deposit(), the tx will revet due to underflow in totalUnderlyingMinusSponsored().

Recommendation

Change to:

function totalUnderlyingMinusSponsored() public view returns (uint256) {
    uint256 _totalUnderlying = totalUnderlying();
    if (totalSponsored > _totalUnderlying) {
        return 0;
    }
    return _totalUnderlying - totalSponsored;
}