Skip to content
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

UniswapHandler.maltMarketPrice returns wrong decimals #255

Open
code423n4 opened this issue Dec 1, 2021 · 1 comment
Open

UniswapHandler.maltMarketPrice returns wrong decimals #255

code423n4 opened this issue Dec 1, 2021 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

cmichel

Vulnerability details

The UniswapHandler.maltMarketPrice function returns a tuple of the price and the decimals of the price.
However, the returned decimals do not match the computed price for the else if (rewardDecimals < maltDecimals) branch:

else if (rewardDecimals < maltDecimals) {
  uint256 diff = maltDecimals - rewardDecimals;
  price = (rewardReserves.mul(10**diff)).mul(10**rewardDecimals).div(maltReserves);
  decimals = maltDecimals;
}

Note that rewardReserves are in reward token decimals, maltReserves is a malt balance amount (18 decimals).
Then, the returned amount is in rewardDecimals + diffDecimals + rewardDecimals - maltDecimals = maltDecimals + rewardDecimals - maltDecimals = rewardDecimals.
However decimals = maltDecimals is wrongly returned.

Impact

Callers to this function will receive a price in unexpected decimals and might inflate or deflate the actual amount.
Luckily, the AuctionEscapeHatch decides to completely ignore the returned decimals and as all prices are effectively in rewardDecimals, even if stated in maltDecimals, it currently does not seem to lead to an issue.

Recommendation

Fix the function by returning rewardDecimals instead of maltDecimals in the rewardDecimals < maltDecimals branch.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 1, 2021
code423n4 added a commit that referenced this issue Dec 1, 2021
@0xScotch 0xScotch added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 8, 2021
@GalloDaSballo
Copy link
Collaborator

Finding is valid, because the returned value is unused, I agree with the medium severity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants