-
Notifications
You must be signed in to change notification settings - Fork 214
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
Curio attack #72
base: master
Are you sure you want to change the base?
Curio attack #72
Conversation
|
||
function test_attack() public { | ||
console.log("\n==== STEP 0: Instance protocol contracts ===="); | ||
// These contracts were deployed almost 3 years before the attack by Curio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attack by Curio?
Curio Finance leverages MakerDAO contracts for managing specific protocol functionalities. Their CSC Curio token, a `DSToken`, designates the `DSPauseProxy` contract as its minter. The `DSPause` contract relies on `DSChief.canCall` to verify if an authorized (privileged) call is permitted. | ||
Then, executes a `delegatecall` through its proxy. | ||
|
||
The operational vulnerability resided in the `DSChief` contract, which manages a HAT account that can be reconfigured with not much voting power. The attacker exploited this by locking tokens in `DSChief`, getting enough votes to lift and displace the current hat, subsequently positioning their contract as the new hat. The main issue was that the previous hat (Curio's `DSSSpell`) did not have enough voting power to consider the system safe, which allowed the easy and cheap displacement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which manages a HAT account that can be reconfigured with not much voting power
Can be reconfigured or was configured?
The main issue was that the previous hat (Curio's `DSSSpell`) did not have enough voting power to consider the system safe
Similarly, consider the system safe is what regard? If DSSSpell
had enough voting power, what would have been prevented?
I think this paragraph can benefit from removing the "cans" and other generally meaningless adjectives (eg: "safe") and replace them with facts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More: the paragraph starts with "the operational vulnerability resided in the DSCHief
..." but then later "the main issue was that the previous hat". So which one is it?
|
||
## Detailed Description | ||
Curio Finance leverages MakerDAO contracts for managing specific protocol functionalities. Their CSC Curio token, a `DSToken`, designates the `DSPauseProxy` contract as its minter. The `DSPause` contract relies on `DSChief.canCall` to verify if an authorized (privileged) call is permitted. | ||
Then, executes a `delegatecall` through its proxy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird wording. "Then"? After what? If the call is permitted?
|
||
## Step-by-step | ||
1. The attacker locked tokens into the `DSChief` contract. | ||
2. Accumulated enough voting power. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The voting power is accumulated through step (1)? Or is it a separate step?
- **Date:** Mar 23, 2024 | ||
- **Reproduce:** `forge test --match-contract=Exploit_Curio -vvv` | ||
|
||
## Step-by-step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The step-by-step is weird because it describes what happens in the contract, instead of the step-of-step for the attack, as usual.
} | ||
``` | ||
|
||
Upon making that `delegatecall`, `DSPause` verifies through `DSChief.canCall`, which returned true given the attacker's contract was now the hat. Since `DSPause` executes actions with `delegatecall` and got minting authority for the token, the attacker deployed a malicious custom `Spell` contract to mint tokens directly to their address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon making... DSPause
verifies thorugh...
What does DSPause
verify?
No description provided.