code-423n4 / 2021-10-ambire-findings

0 stars 0 forks source link

`Zapper` withdrawals from Aave might not work as intended #34

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

cmichel

Vulnerability details

As I understand it, the Zapper is supposed to be called as part of a batch transaction originating from the user or identity contract. But when calling lendingPool.withdraw, for example in exchangeV2, Aave tries to withdraw from the Zapper balance, not the user balance. There should never be a collateral balance for the Zapper contract as anyone could withdraw it at any time.

Impact

It's unclear why withdrawing from the Lending pool as the Zapper account would ever be useful. The intention might have been to withdraw as the caller msg.sender but that doesn't currently work.

Recommended Mitigation Steps

Clarify the intention of this Aave withdrawal, and remove the code if it's not needed or if the intention was to withdraw as the user not as the Zapper contract.

Ivshti commented 2 years ago

the usage is correct: as part of a batch transaction that executed from the Identity, a user would first send an aToken to the zapper

then, the withdraw there is used to unwrap this aToken back to the original and trade it

GalloDaSballo commented 2 years ago

As per the sponsor, the zapper expects funds to be there as part of a batch transaction