In Vault.sol, totalUnderlying() (and, therefore, totalUnderlyingMinusSponsored()) include both funds available in the Vault as well as those invested in the Strategy. The calculation of amounts returned to depositors and sponsors (in _withdraw() and _unsponsor()) also relies on the values produced by totalUnderlying()/minusSponsored() which include invested funds that are not present in the Vault contract.
Therefore, even if the following check in _unsponsor() succeeds:
require(sponsorToTransfer <= totalUnderlying(), "Vault: not enough funds to unsponsor");
That is further complicated by the fact that the Vault does not implement any ability to pull funds from the Strategy.
## Proof of Concept
Vault.totalUnderlying():
https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L125
Vault.totalUnderlyingMinusSponsored():
https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L292
_unsponsor():
https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L394
_withdraw():
https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L344
## Tools Used
Manual Analysis
## Recommended Mitigation Steps
During withdraw()/unsponsor() check if the amount to be withdrawn is <= the amount of underlying *in possession of the Vault*. Consider adding functionality that allows pulling *some* funds out of the strategy, when necessary.
r2moon
commented
2 years ago
Handle
palina
Vulnerability details
Impact
In Vault.sol, totalUnderlying() (and, therefore, totalUnderlyingMinusSponsored()) include both funds available in the Vault as well as those invested in the Strategy. The calculation of amounts returned to depositors and sponsors (in _withdraw() and _unsponsor()) also relies on the values produced by totalUnderlying()/minusSponsored() which include invested funds that are not present in the Vault contract. Therefore, even if the following check in _unsponsor() succeeds:
require(sponsorToTransfer <= totalUnderlying(), "Vault: not enough funds to unsponsor");