Malicious user can still native tokens of MoneyMarketHook caller

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L76-L80

Vulnerability details

Proof of Concept

MoneyMarketHook allows user to chain some actions into one multicall to the InitCore. In the end user can get all wrapped native tokens that he withdrew in a form of native token. Note, that this part of code withdraws all balance from wrapped token and the sends all balance to the msg.sender. In case if balance of contract is 0, then the call will still succeed.

One type of actions that caller can do using execute function is withdraw. In case if caller needs to repay funds to other address then he can provide it as to param. So as result, IInitCore.burnTo will transfer pool tokens to the provided recipient. And in case if token has a hook, like erc777 or erc677, then receiver will be triggered and he has execution control now.

User can have multiple withdraws in same multicall. And it's possible that first withdraw is wrapped native token withdraw(which user would like to receive as native) and next withdraw is with erc777 token and other recipient.

In such case, when first withdraw is done, then wrapped token is already on MoneyMarketHook balance. And then when second withdraw is handled and attacker get hook, then he can call MoneyMarketHook.execute(it doesn't have reentrancy check) again with any params that just should pass and _params.returnNative as true. Then all wrapped token balance will be sent to the attacker and then execution will return back to the victim and it will send 0 amount as native token to the victim.

As result attacker is able to steal user's native tokens.

From readme it's clear that only fee on transfer tokens are not supported and erc777 are supported:

Please list specific ERC20 that your protocol is anticipated to interact with. Could be "any" (literally anything, fee on transfer tokens, ERC777 tokens and so forth) or a list of tokens you envision using on launch. No fee-on-transfer tokens

Impact

It's possible to steal user's funds.

Tools Used

VsCode

Recommended Mitigation Steps

Add reentrancy check to the execute function.

Assessed type

Error

[c4-judge]

hansfriese marked the issue as primary issue

[c4-sponsor]

fez-init marked the issue as disagree with severity

[fez-init]

This should be a medium issue. There are multiple things that needs to be aligned, including

• supporting of ERC777. We did not intend to support ERC777, but acknowledge that it may not be communicated clearly.
• the victim needs to specify a certain malicious to contract for the attack to happen.

[fez-init]

We will add reentrancy check to the execute function to prevent this kind of scenario anyways.

[c4-sponsor]

fez-init (sponsor) confirmed

[hansfriese]

I agree Medium is appropriate due to this requirement. - the victim needs to specify a certain malicious to contract for the attack to happen.

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese changed the severity to 2 (Med Risk)

[c4-judge]

hansfriese marked the issue as selected for report