-
Notifications
You must be signed in to change notification settings - Fork 2
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
zraxx - Malicious users can gain more vote shares after voteFactor
is changed.
#37
Comments
voteFactor
is changed.voteFactor
is changed.
If admin action can break certain assumptions about the functioning of the code, it's not considered a valid issue |
I know what you mean, my understanding is that we should trust the admin actions. But here, the admin action will lead to the violation of the protocol in any case. Isn't it a issue? |
@zrax-x could you please give me links to |
@WangSecurity Thank you for your attention. The link of function // Update voting power based on distribution record initialization or claims
function _reconcileVotingPower(address beneficiary) private {
// current potential voting power
uint256 currentVotes = balanceOf(beneficiary);
// correct voting power after initialization, claim, or adjustment
DistributionRecord memory record = records[beneficiary];
uint256 newVotes = record.claimed >= record.total ? 0 : tokensToVotes(record.total - record.claimed);
if (currentVotes > newVotes) {
// reduce voting power through ERC20Votes extension
_burn(beneficiary, currentVotes - newVotes);
} else if (currentVotes < newVotes) {
// increase voting power through ERC20Votes extension
_mint(beneficiary, newVotes - currentVotes);
}
} Here, the number of votes is obtained through the As for function function _executeClaim(
address beneficiary,
uint256 _totalAmount,
bytes memory data
) internal virtual returns (uint256) {
uint120 totalAmount = uint120(_totalAmount);
// effects
if (records[beneficiary].total != totalAmount) {
// re-initialize if the total has been updated
_initializeDistributionRecord(beneficiary, totalAmount);
}
uint120 claimableAmount = uint120(getClaimableAmount(beneficiary, data));
require(claimableAmount > 0, 'Distributor: no more tokens claimable right now');
records[beneficiary].claimed += claimableAmount;
claimed += claimableAmount;
return claimableAmount;
} It is worth noting that functions |
zraxx
medium
Malicious users can gain more vote shares after
voteFactor
is changed.Summary
Malicious users can gain more votes after
voteFactor
is changed.Vulnerability Detail
In the current protocol design, the user's vote count will be updated during
initializeDistributionRecord
andclaim
. It is worth noting that there is no access control for these two methods. The owner can changevoteFactor
, but the user's vote count will not be directly affected (instead will take effect the next timeinitializeDistributionRecord
orclaim
is called). Therefore, an attacker can use this to increase his or her vote share.When
voteFactor
is reduced, the attacker calls theclaim
function with other beneficiaries’ address as arguments, thereby reducing their vote count. When thevoteFactor
increases, the user calls theinitializeDistributionRecord
function with his/her own address. Ultimately in both cases the attacker's vote share will be increased.Impact
Malicious users could increase their vote shares.
Code Snippet
https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/abstract/AdvancedDistributor.sol#L191-L194
https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/abstract/AdvancedDistributor.sol#L78-L92
Tool used
Manual Review
Recommendation
Change the way the user's votes are calculated. Use balanceOf to get the shares, and then convert to votes using tokensToVotes. In this way, the user's vote count will take effect immediately after the voteFactor changes, instead of requiring the user to actively trigger an update (i.e., call function initialize or claim).
For example, use this way to get votes:
Then the function _reconcileVotingPower will be:
The text was updated successfully, but these errors were encountered: