Opus - ABAIKUNANBAEV's results
A cross margin credit protocol with autonomous monetary policy and dynamic risk parameters.
General Information
Platform: Code4rena
Start Date: 09/01/2024
Pot Size: $100,000 USDC
Total HM: 13
Participants: 28
Period: 28 days
Judge: 0xsomeone
Total Solo HM: 8
Id: 319
League: ETH
Findings Distribution
Researcher Performance
Rank: 5/28
Findings: 1
Award: $4,529.91
π Selected for report: 1
π Solo Findings: 1
Findings Information
π Selected for report: ABAIKUNANBAEV
Labels
Awards
4529.9124 USDC - $4,529.91
External Links
Lines of code
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L167
Vulnerability details
Impact
In Controller smart contract, multiplier value should be calculated using this formula:
However, due to mistake in multiplication of i_gain, it's miscalculated and the error occurs. This can lead to a multiplier reflecting an incorrect value and therefore affect yin borrowing rate.
Proof of Concept
According to the multiplier formula, first we calculate p_term. The formula for p_term is following:
p_term = p_gain * error
And that's done right using the get_p_term_internal() function:
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L243-245
fn get_p_term_internal(self: @ContractState) -> SignedRay { self.p_gain.read() * nonlinear_transform(self.get_current_error(), self.alpha_p.read(), self.beta_p.read()) }
Then we add right and go to integral term calculation. It's almost the same as p_term - only time scale factor is added:
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L256-258
old_i_term + nonlinear_transform(self.get_prev_error(), self.alpha_i.read(), self.beta_i.read()) * time_since_last_update_scaled
The problem is that when we add this formula to the multiplier, we multiply it additionally by i_gain:
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L166-167
let new_i_term: SignedRay = self.get_i_term_internal(); multiplier += i_gain * new_i_term;
And, according to the formula, only this part should be multiplied by i_gain:
nonlinear_transform(self.get_prev_error(), self.alpha_i.read(), self.beta_i.read()) * time_since_last_update_scaled
But old_term also ends up multiplied.
Tools Used
Manual review.
Recommended Mitigation Steps
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L256-258
- old_i_term + nonlinear_transform(self.get_prev_error(), - self.alpha_i.read(), self.beta_i.read()) * - time_since_last_update_scaled + old_i_term + i_gain * nonlinear_transform(self.get_prev_error(), + self.alpha_i.read(), self.beta_i.read()) * + time_since_last_update_scaled
https://github.com/code-423n4/2024-01-opus/blob/main/src/core/controller.cairo#L167
- multiplier += i_gain * new_i_term; + multiplier += new_i_term;
Assessed type
Other
#0 - bytes032
2024-02-10T06:54:52Z
The source for the formula is not available
#1 - c4-pre-sort
2024-02-10T06:54:55Z
bytes032 marked the issue as insufficient quality report
#2 - alex-ppg
2024-02-26T14:27:05Z
The Warden has demonstrated how the formula that is in use by the Controller contradicts the documentation of the project, and namely the specification of the aforementioned formula.
In detail, the specification states:
We are interested in the latter part of the formula, specifically:
The documentation states that $k_i$ stands for the:
gain that is applied to the integral term
The problem highlighted by the Warden is that the implementation will calculate the following:
The above corresponds to:
let new_i_term: SignedRay = self.get_i_term_internal(); multiplier += i_gain * new_i_term;
Whereby the get_i_term_internal function will ultimately yield:
old_i_term + nonlinear_transform(self.get_prev_error(), self.alpha_i.read(), self.beta_i.read()) * time_since_last_update_scaled
In the above:
i_gain: $k_i$old_i_term: $y_i[k-1]$nonlinear_transform(self.get_prev_error(), self.alpha_i.read(), self.beta_i.read()): $\frac{u[k-1]^{a_{ki}}}{{\sqrt{1 + u[k-1]^{Ξ²_{ki}}}}}$time_since_last_update_scaled: $\Delta t$
Based on the above analysis, the documentation indeed contradicts what the code calculates. The Opus team is invited to evaluate this exhibit, however, regardless of the correct behaviour of the system this submission will be accepted as valid due to the documentation being the "source of truth" during the contest's duration.
#3 - c4-judge
2024-02-26T18:15:43Z
alex-ppg marked the issue as satisfactory
#4 - c4-judge
2024-02-26T18:15:46Z
alex-ppg marked the issue as selected for report
#5 - c4-sponsor
2024-02-27T15:01:46Z
milancermak (sponsor) confirmed
#6 - rodiontr
2024-02-27T19:31:17Z
@alex-ppg sorry, but why itβs marked as insufficient quality ? I donβt know whatβs the problem with the source but I can see the formula when I open up the issue
#7 - alex-ppg
2024-02-29T15:29:00Z
Hey @rodiontr, thanks for supplying feedback on this submission. The source is visible to you because you used a personal link that is private for all other GitHub users:
https://github.com/rodiontr/fdkfd/assets/109477234/c9f8db25-3b94-4662-b86a-1541a1fc133c
You should try utilizing other image hosting platforms in the future if you wish your images to be visible. Additionally, the submission has been accepted as valid, and the Insufficient Quality Report label is one assigned by the pre-sorter rather than the judge. It does not impact the submission's payout.