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

fix h01: address issue in recoverErc20 function #5

Merged
merged 1 commit into from
May 17, 2022

Conversation

chrismaree
Copy link
Member

@chrismaree chrismaree commented May 17, 2022

Problem:

H-01 Anyone can prevent stakers from getting their rewards
The recoverErc20 function is meant to facilitate the recovery of any ERC20 tokens that maybe mistakenly sent to the AcceleratingDistributor contract. As this is a public function
with no modifiers, anyone can call this function to transfer an ERC20 token from the AcceleratingDistributor contract to the owner of AcceleratingDistributor . The
only ERC20 tokens that are explicitly disallowed from being recovered are stakedToken that have already been initialized in the system.

However, it is currently possible to recover the ERC20 rewardToken using the recoverErc20 function. Doing so would transfer some specified amount of rewardToken
from the AcceleratingDistributor contract to the contract's owner. This would, subsequently, prevent stakers from being able to access their rewards because
AcceleratingDistributor could be left with an insufficient balance of rewardToken s.

Even if the owner were to send rewardToken s back to the AcceleratingDistributor
contract, a malicious actor could immediately transfer all of the rewardTokens back to the
owner. Redeployment would be necessary to fix the issue.

Solution:

An additional modifier was added to check the tokenAddress in the recoverErc20 method is not the reward token. Tests were also updated

Signed-off-by: chrismaree <christopher.maree@gmail.com>
@chrismaree chrismaree merged commit e65f487 into master May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants