-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Percentage calculation could leave unused ETH leftovers in AfEth deposit #40
Comments
0xleastwood marked the issue as primary issue |
0xleastwood marked the issue as selected for report |
elmutt (sponsor) confirmed |
@0xleastwood, Just wanna understand the impact behind this issue. Is it possible to run into the case, where the leftover will be more than a small The invariant is not broken(e.g user funds are 100% allocated between strategies) |
Yes it would be. We have two calculations |
Okay tested this out a bit, it appears rounding is capped at 1 wei. |
Downgrading to QA because this is super minor. |
0xleastwood marked the issue as not selected for report |
0xleastwood changed the severity to QA (Quality Assurance) |
Thanks for flagging this. |
Lines of code
https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L160
Vulnerability details
Summary
The logic used to split the deposit amount between SafEth and the Votium strategy can leave some leftovers that will be ignored by the implementation.
Impact
Deposits in the AfEth contract are split based on a configured
ratio
. One portion of the split goes to SafEth, while the other is deposited in the Votium strategy.To calculate the split, the implementation multiplies the deposit amount by
ratio
for the SafEth portion, and the deposit amount by1e18 - ratio
for the VotingStrategy portion.https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L148-L169
Because of rounding and limited precision integer math, the split could potentially not cover the entirety of the deposit, i.e.
sValue + vValue != amount
. There could be some leftovers from both calculations that won't be considered in the deposit.Recommendation
Instead of calculating
vValue
using1e18 - ratio
, simply use difference between the deposited amount and the calculated SafEth amount (sValue
).Assessed type
Other
The text was updated successfully, but these errors were encountered: