Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

yixxas - redeem() may not withdraw the correct amount of underlying token for caller for the number of uTokens burned #33

Closed
sherlock-admin opened this issue Nov 4, 2022 · 0 comments

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Nov 4, 2022

yixxas

medium

redeem() may not withdraw the correct amount of underlying token for caller for the number of uTokens burned

Summary

A user who wishes to redeem uTokens for underlying asset can callredeem(). However, assetManagerContract.withdraw() does not guarantee that the amount withdrawn is always equal to the underlyingAmount.

Vulnerability Detail

The vulnerablity is due to the withdraw() function in AssetManager. It first checks for selfBalance and then loops through all moneyMarket to look for the amount that is requested to be withdrawn. There is no guarantee that at the end of the function, remaining == 0. This can happen due to many reasons.
Note that in redeem() in uToken.sol, the only condition checked for allowed underlyingAmount is totalRedeemable -= underlyingAmount, where function will underflow if underlyingAmount > totalRedeemable.
AssetManager deposits its funds into money markets such as Aave or Compound protocol. There is lending risk and smart contract risk in the money markets, and as more such markets are added, the risk increases. The total assets available for withdraw may not match the amount accounted for in totalRedeemable.
withdraw() returns true even if remaining > 0, so it does not revert, yet the number of uTokens burned is equivalent to the fully redeemed amount for the caller in redeem() in the UToken.sol contract.

Impact

Users will burn their uTokens, and yet not able to redeem the correct number of underlying tokens.

Code Snippet

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/asset/AssetManager.sol#L328-L369
https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/market/UToken.sol#L707-L740

Tool used

Manual Review

Recommendation

withdraw() should return false if remaining > 0 to signify that the full amount was not successfully withdrawn.

Duplicate of #27

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant