Closed c4-bot-4 closed 5 months ago
Picodes marked the issue as duplicate of #553
Picodes marked the issue as satisfactory
Picodes marked the issue as selected for report
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
This is a good point and we will fix, but not sure it fulfills Medium severity given the lack of impact (& CollateralTracker is not on the compliance checklist).
The only impact of this is that if, for some reason, users attempt to mint with the result of maxMint
(which is an unrealistic amount of tokens for most, if not all actively traded tokens), their transaction will revert (in which case they could just mint slightly less according to the "real" maxMint
).
Even if maxMint
was correct as to spec, their transaction would similarly revert if the share price changed before their transaction was included.
Hello Judge, in case this remains as medium, I'd like to ask this finding from my QA report to be marked as a duplicate of this report.
In that finding, I explained the same issue reported in this report.
As the contract is not in the compliance checklist, the argument for med "broken functionality", the functionality being the compliance to the EIP doesn't hold, so QA is more appropriate.
Picodes changed the severity to QA (Quality Assurance)
Picodes marked the issue as not selected for report
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L444-L448
Vulnerability details
Impact
CollateralTracker is not EIP4626 compliant. Specifically, the
maxMint
is calculated to be too large, and users will fail minting the sharesmaxMint
returns.Bug Description
First, let's quote the EIP4626 doc https://eips.ethereum.org/EIPS/eip-4626:
The
maxMint
value should never be overestimated, and user should always be able to mint the amount of assetsmaxMint
returns.However, this is not the case for CollateralTracker. For CollateralTracker, and the share to mint formula (by
previewMint
) is:assets = shares * DECIMALS * totalAssets / (totalSupply * (DECIMALS - COMMISSION_FEE))
.From which we can derive
shares = assets * (totalSupply * (DECIMALS - COMMISSION_FEE)) / (totalAssets * DECIMALS)
, and given the maximum assets isuint104.max
, the correct maximum shares (maxMint) should beconvertToShares(type(uint104).max * (DECIMALS - COMMISSION_FEE)) / DECIMALS
which is smaller than the currentmaxMint()
.Proof of Concept
Add the following test code in
CollateralTracker.t.sol
. See that we try to mintmaxMint()
even with a 1e18 buffer, but it still fails.Tools Used
Foundry
Recommended Mitigation Steps
Use
convertToShares(type(uint104).max * (DECIMALS - COMMISSION_FEE)) / DECIMALS
formaxMint
function.Assessed type
Other