Flow
Sablier
FoundryDeFi
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Admin can Drain Token Funds due to Multiple Token Addresses.

mrkaplan

Vulnerability Details

The SablierFlowBase::recover function is designed to recover funds that have been accidentally sent to the contract. It calculates the contract's token balance, subtracts the aggregate balance of that same token, and sends the remaining difference to an address specified in the function's parameters.

function recover(IERC20 token, address to) external override onlyAdmin {
uint256 surplus = token.balanceOf(address(this)) - aggregateBalance[token];
​
// Check: there is a surplus to recover.
if (surplus == 0) {
revert Errors.SablierFlowBase_SurplusZero(address(token));
}
​
// Interaction: transfer the surplus to the provided address.
token.safeTransfer(to, surplus);
​
emit Recover(msg.sender, token, to, surplus);
}

Note: The aggregateBalance is a mapping (mapping(IERC20 token => uint256 amount)) that tracks the total balances of the every token that interacts with the contract functions.

The issue arises when a token has multiple addresses through which its functions can be called. In this situation, the protocol's admin could potentially drain the entire balance of a token by using an address that was not originally employed to create the streams.

Note: Currently, I am not aware of any tokens with this behavior, but it should be considered because such tokens could emerge in the future. Since the tokens used in the streams are selected by regular users, it's important to account for this possibility.

Additionally, it could lead to the creation of multiple aggregate balances for the same token.

Impact

  • Drain the contract funds of a specific token.

  • Creation of multiple aggregate balances for the same token.

Recommendations

Add a function to block specific addresses of tokens. This will allow you to block secondary addresses when such tokens are introduced in the future.

​
+ mapping(IERC20 token => bool) blockedMultipleAddress;
​
+ function blockMultipleAddressToken(IERC20 token, bool allowed) external onlyAdmin {
+ multipleAddress[token] = allowed;
+ }

Create a modifier:

+ modifier notBlockedAddress(IERC20 token) {
+ if (multipleAddress[token]) {
+ revert "Multiple address not allowed";
+ }
+ _;
+ }

Add it to the function:

+ function recover(IERC20 token, address to) external override onlyAdmin notBlockedAddress(token) {
- function recover(IERC20 token, address to) external override onlyAdmin {
// ...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

mrkaplan Submitter
8 months ago
inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.