Skip to content
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

Due to loss of precision, targetVotes may not reach #59

Open
code423n4 opened this issue Oct 29, 2022 · 3 comments
Open

Due to loss of precision, targetVotes may not reach #59

code423n4 opened this issue Oct 29, 2022 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-01 satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

code423n4 commented Oct 29, 2022

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L245-L246

Vulnerability details

Impact

In the _pledge function, require delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount <= pledgeParams.targetVotes.
In reality, when the user pledges amount votes, the actual votes received by the receiver are the bias in the following calculation. And the bias will be less than amount due to the loss of precision.

        uint256 slope = amount / boostDuration;
        uint256 bias = slope * boostDuration;

This means that the balance of receiver may not reach targetVotes

    point = self._checkpoint_read(_user, False)
    amount += (point.bias - point.slope * (block.timestamp - point.ts))
    return amount

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L245-L246
https://github.com/curvefi/curve-veBoost/blob/master/contracts/BoostV2.vy#L192-L209
https://github.com/curvefi/curve-veBoost/blob/master/contracts/BoostV2.vy#L175

Tools Used

None

Recommended Mitigation Steps

Use bias instead of amount in the check below

        uint256 slope = amount / boostDuration;
        uint256 bias = slope * boostDuration;
        if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + bias > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();
        delegationBoost.boost(
            pledgeParams.receiver,
            amount,
            endTimestamp,
            user
        );
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 29, 2022
code423n4 added a commit that referenced this issue Oct 29, 2022
@Kogaroshi Kogaroshi added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Oct 30, 2022
@Kogaroshi
Copy link

The current check is made that way to prevent any unnecessary call to the BoostV2 contract (and save gas by not creating the Boost) in the case of a targetVotes overflow.

@c4-judge
Copy link
Contributor

kirk-baird marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Nov 11, 2022
@trust1995
Copy link

I believe there is no actual M level issue here, the worst case scenario is an inconvenience so that user has to pass bias (i.e. rounded amount) instead of amount (I presume this will already occur in the front end). No risk of funds or malfunctioning of the protocol shown.

@C4-Staff C4-Staff added M-01 selected for report This submission will be included/highlighted in the audit report labels Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-01 satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

5 participants