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.

Sign up for GitHub

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

Jump to bottom

rewardTokens can be removed by owner without claiming them first #100

Closed
code423n4 opened this issue Nov 24, 2022 · 5 comments
Closed

rewardTokens can be removed by owner without claiming them first #100

code423n4 opened this issue Nov 24, 2022 · 5 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 downgraded by judge Judge downgraded the risk level of this issue duplicate-271 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexRewards.sol#L179
https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexRewards.sol#L387

Vulnerability details

Impact

Any unclaimed rewardTokens will be unclaimable once a rewardToken has been removed from producerTokens[producerToken].rewardTokens list.

Proof of Concept

The owner can call removeRewardToken to remove any rewardToken from a producerToken. However, if the rewardToken still has unclaimed rewards, as evidenced by producerTokens[producerToken].rewardStates, these rewards will be unclaimable.

The claim function, once identified the producerToken, loops through rewardTokens list and claims rewards as marked in rewardStates[rewardToken]. However, if a rewardToken has been removed, even if rewardStates[rewardToken] is non-zero, it would not be claimable.

Tools Used

manual

Recommended Mitigation Steps

Check require(rewardStates[rewardTokens[removalIndex]] == 0) before removing the unwanted rewardToken.
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 24, 2022
code423n4 added a commit that referenced this issue Nov 24, 2022
@code423n4
datapunk issue #100
c460249
@c4-judge c4-judge closed this as completed Dec 3, 2022
@c4-judge
Copy link
Contributor

c4-judge commented Dec 3, 2022

Picodes marked the issue as duplicate of #255

1 similar comment
@c4-judge
Copy link
Contributor

c4-judge commented Dec 5, 2022

Picodes marked the issue as duplicate of #255

@c4-judge
Copy link
Contributor

c4-judge commented Jan 1, 2023

Picodes marked the issue as duplicate of #271

@c4-judge
Copy link
Contributor

c4-judge commented Jan 1, 2023

Picodes changed the severity to 2 (Med Risk)

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 1, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Jan 1, 2023

Picodes marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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 downgraded by judge Judge downgraded the risk level of this issue duplicate-271 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@code423n4 @c4-judge