search

code-423n4 / 2021-06-pooltogether-findings

0 stars 0 forks source link

ATokenYieldSource mixes aTokens and underlying when redeeming #86

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The ATokenYieldSource.redeemToken function burns aTokens and sends out underlying, however, it's used in a reverse way in the code: The balanceDiff is used as the depositToken that is transferred out but it's computed on the aTokens that were burned instead of on the depositToken received.

Impact

It should not directly lead to issues as aTokens are 1-to-1 with their underlying but we still recommend doing it correctly to make the code more robust against any possible rounding issues.

Recommended Mitigation Steps

Compute balanceDiff on the underyling balance (depositToken), not on the aToken. Subtract the actual burned aTokens from the user shares.

PierrickGT commented 3 years ago

I agree that we should compute balanceDiff on the underlying balance. Regarding the burn of the user's shares, we should keep it as is to verify first that the user has enough shares. This way if he doesn't, the code execution will revert before funds are withdrawn from Aave.

PierrickGT commented 3 years ago

PR: https://github.com/pooltogether/aave-yield-source/pull/17