Open c4-bot-6 opened 9 months ago
Picodes changed the severity to QA (Quality Assurance)
Downgrading to QA as this is more a criticism of the airdrop design than a security finding
hey @Picodes , I disagree that this finding is nothing but criticism on the airdrop design. The core of the issue is that an entity that gains a majority voting power at a given point in time can use that power to gain permanent control over the DAO (shown in the POC). The airdrop is brought here to show a scenario where gaining such a majority is relatively likely (though there could be other scenarios). Most DAO designs (including SALTY's) place limitations on what a DAO can do with a majority vote. Spefically, barriers that prevent a temporary majority entity from cementing their majority (granting themselves permanent control over the dao which goes against the whole purpose of a DAO).
@nirohgo I apologize but I still don't see the point. You are saying that whales could take over a DAO but this is the case everywhere, no? And like if you have a majority of the votes you should be able to pull the funds out and basically do anything as you are the sole owner of the DAO?
I understand that you advocate for stricter control of this and would like to make sure the majority isn't "temporary" but do not see how this is a security finding.
Lines of code
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ExchangeConfig.sol#L74 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Staking.sol#L43
Vulnerability details
DAO can be hijacked by an exploiter who gets a large share of the airdrop (i.e. through a sybil attack) by replacing the AccesManager contract
Github Links
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/AccessManager.sol#L74C14-L74C30 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ExchangeConfig.sol#L74 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Staking.sol#L43
Impact
High Level
An exploiter who gets a large share of airdropped SALT (using bots to spam the airdrop for example) can later dominate the DAO permanently by voting to change the AccessControl contract to a contract they control and then blocking further staking that can jeopardize their voting dominance.
details
The criteria for airdrop eligibility is being in a permitted geo location, tweeting about the airdrop and voting on the bootstrap ballot. This introduces the risk of an airdrop sybil attack - meaning an exploiter can use a twitter bot network/multiple addresses to get a dominant share of the airdrop token. This type of attack is not uncommon in airdrops (see example here) and since Salty's bootstrap ballot does not require a minimum number of participants, will be easier/cheaper to achieve the less participants there are in the airdrop.
An exploiter who achieves ownership of a large share of airdropped XSalt will receive the funds immediately when the system is launched and can use them to vote on changing the AccessManager contract, which effectively gives them a way to maintain voting dominance by monitoring the mempool for Salt staking transactions and blocking them if the staked Salt amount causes them to lose voting dominance.
Considering the time delays to achieve an AccessManager contract change (45 days before ballots can be opened + 10 days for first ballot + 10 days for confirmation ballot = 65 days) and the rate of SALT emissions to recipients, it can be shown (see POC below) that with a large airdrop share (~>53%) it is impossible to block the change proposal, and unlikely even with a smaller share (>=30%).
The following attack scenario and POC demonstrate how this can be done:
exploit scenario
An exploiter manages to gain control of a large share of the airdrop funds by using multiple twitter accounts/addresses (66% in the POC).
After the 45 days required to propose a ballot, the exploiter proposes a setContract ballot to replace the AccessControl contract. This will require an additional 20 days, 10 to pass the initial proposal, and another 10 to pass the proposal confirmation ballot, bringing the period the exploiter needs to maintain voting dominance to a total of 65 days.
The POC below shows how an exploiter with a majority XSalt at the time of the airdrop can maintain voting majority after 65 days. This is shown by calculating the ratio of exploiter XSalt to the maximum amount of available XSalt/Salt that can be used to oppose the proposal, at the time of each vote. The maximum amount includes: all airdropped XSalt, all emitted staking and liquidity rewards at the time of each vote and all team vested Salt distribution (considering the team may use these fund to vote against the proposal). Dao vested allocation was excluded since the DAO can't vote directly and will require a separate ballot to transfer its funds to someone who can vote.
In the POC the exploiter starts with 66.6% of the airdrop Salt, and maintains 64.65% majority by the time the AccessControl contract is replaced. This indicates that an even smaller majority (estimated around 53%) will maintain >50% voting power that guarantees the exploit. In reality, even less voting power may be enough (as little as the 30% quorum required to pass a setContract) given that voter participation (especially at the early days of the DAO) is typically not very high and that not all Salt emitted rewards will be collected.
Once the ballot is approved, the attacker can use the rogue AccessControl contract to block any further staking that might jeopardize their dominance. To do this, the rogue AccessControl contract can have a "block(wallet)" transaction (available to the exploiter only) that causes the AM to report false on walletHasAccess for a specific wallet. The exploiter can send this transaction to front run any Salt staking transaction of an amount that puts their voting dominance at risk.
At this point the attacker controls the system and has the capacity for exploits such as: propose and approve sending all DAO salt to themselves, whitelist rogue tokens that enable exploiting user funds etc.
If/when other users (airdrop users or new Salty users) notice the exploit, the only remedy is to restart DAO from scratch, effectively DOSing Salty for the duration of time it will take to change the code and run a new bootstrap ballot.
Proof of Concept
The following POC shows how with 3 airdrop participants (for simplicity) if two are controlled by an exploiter (i.e. 66.6% of the airdrop) they can guarantee maintaining their majority (by collecting and staking all rewards available to them before each vote) long enough to change the AccessControl contract and secure permanent control of the DAO. Permanent control is achieved by causing staking transactions to fail when they see fit, using the set rogue AccessControl contract, controlled by the exploiter.
Running the POC
contract RogueAccessManager is IAccessManager { mapping(address => bool) private _blockWallets; address private owner; constructor( address _owner ) { owner = _owner; } function excludedCountriesUpdated() external {
}
Add to following functions to the TestComprehensive1 contract in '2024-01-salty\src\scenario_tests\Comprehensive1.t.sol'
function printVoteDominance(uint256 ballotID, uint256 daysSinceStart) public {
}
//Passes a vote for the expoiter: function voteAndFinalize(uint256 ballotID) internal { vm.warp(block.timestamp + 606024*10 + 13); //Warp 10 days (minimum voting time) upkeep.performUpkeep(); //Perform upkeep to make sure all scheduled rewards are emitted voteMaxForUser(alice,ballotID); //Have both alice and bob stake all pending rewards and vote
voteMaxForUser(bob,ballotID); dao.finalizeBallot(ballotID); //Finalize Ballot }
function voteMaxForUser(address user, uint256 ballotID) internal { vm.startPrank(user); bytes32[] memory pools = new bytes32; staking.claimAllRewards(pools); salt.approve(address(staking), type(uint256).max); staking.stakeSALT(salt.balanceOf(user)); proposals.castVote(ballotID,Vote.YES); vm.stopPrank(); }