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

chore(contracts): fixed critical security issues #59

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

developerfred
Copy link

Security Fix

Reentrancy Attack Vulnerability

The claimReward() function calls an external contract rewardToken.transfer(), which may cause a reentrancy attack if the external contract is malicious or vulnerable. An attacker can exploit this vulnerability to re-enter the vulnerable function before the execution is complete, and therefore manipulate the contract state.

Denial of Service (DoS) Vulnerability

The isEpochFunded() function may cause a denial of service vulnerability since it loops through all the past epochs to calculate whether the current epoch is funded or not. If the number of past epochs becomes very large, the function may consume an excessive amount of gas and fail to execute, thereby preventing other legitimate functions from executing.

The removePriority() function has been improved by adding a mechanism to prevent the removal of priorities during a voting period, which could potentially lead to vote manipulation.

The onlyOwner modifier has also been omitted, I think we can add the one from OpenZeppelin.

How Has This Been Tested?

Sector#3 Contribution

@developerfred developerfred force-pushed the security/white-hat-fixed branch from c7d937d to 1685ff2 Compare March 14, 2023 13:16
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.

1 participant