code-423n4 / 2024-01-opus-findings

0 stars 0 forks source link

Overly Strict Liquidation Conditions in Purger Causing False Positives #150

Closed c4-bot-8 closed 7 months ago

c4-bot-8 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/roles.cairo#L195-L197 https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/roles.cairo#L185-L187

Vulnerability details

Impact

Least Privilege Principles The roles mostly follow principles of least privilege by granting restricted subsets of permissions: src/core/roles.cairo#L195-L197

src/core/roles.cairo#L185-L187

fn sentinel() -> u128 {
  ADD_YANG + UPDATE_YANG_SUSPENSION 
}

fn purger() -> u128 {
  MELT + REDISTRIBUTE + SEIZE
}

This limits blast radius if a contract with that role is compromised.

Exceptions

A few exceptions granting unnecessary privileges:

Tools Used

Vscode

Recommended Mitigation Steps

🔥 Remove EXIT for Purger in Sentinel

🔥 Restrict all_roles() to test environments

🔒 Add timelock for admin role changes

Assessed type

Access Control

c4-pre-sort commented 7 months ago

bytes032 marked the issue as insufficient quality report

alex-ppg commented 7 months ago

The Warden details how the role structure of the Opus system can be fine-tuned which cannot constitute an HM vulnerability and is better suited in an Analysis report.

c4-judge commented 7 months ago

alex-ppg marked the issue as unsatisfactory: Invalid

c4-judge commented 7 months ago

alex-ppg marked the issue as primary issue

c4-judge commented 7 months ago

alex-ppg marked the issue as unsatisfactory: Invalid