VaultManager.sol - hardcoded receiver could be block-listed

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0xa67aafc2da7356d8b862250b29c302f5400d797bbc1ca56576c5f2f25f15e95f Severity: low

Description: Description\ Popular tokens like USDT and USDC most notably, feature an internal block-list. The issue here lies in the fact that multiTokenWithdrawal hardcodes the receiver as the sender of the message.

Attack Scenario\ As multiTokenWithdrawal calls _multiTokenWithdrawal(msg.sender, msg.sender, _portfolioTokenAmount) with hardocoded amounts, a withdrawing user may be deemed unable to take out his deposit if he is block-listed by the token as he will be unable to receive it. As the block-list impacts only the blocked user, but his funds would still be in circulation and impact ratios one way or another, I went with Low

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Recommendation\ Allow the withdrawing user to specify a custom receiver address, instead of hardcoding to msg.sender

[0xWeb3boy]

A user always has the option to withdraw his funds using multiTokenWithdrawalFor() function, in that case no harm is being done since he can always pass another address to receive his tokens.

[langnavina97]

This issue is out of scope and beyond our control. The internal block-list of tokens like USDT and USDC is a constraint set by those tokens themselves, and we cannot modify this behavior. Allowing the withdrawing user to specify a custom receiver address would not solve the fundamental limitation imposed by the token contracts.

@PlamenTSV

[PlamenTSV]

It is more of a quality of life improvement, since it would still allow circulating tokens to be successfully withdraw out of the system, since currently you cannot remove those tokens and they would always inflate the balances and supplies. These are generally low issues