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

QA Report #304

Open
code423n4 opened this issue May 25, 2022 · 0 comments
Open

QA Report #304

code423n4 opened this issue May 25, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Low

  1. Title : using pragma abicoder V2

Since AuraLocker.sol was using pragma experimental ABIEncoderV2 and used pragma ^0.8.11, it because the ABI coder v2 is not considered experimental anymore, it can be selected via pragma abicoder v2 instead since Solidity 0.7.4.

##POC

The pragma pragma experimental ABIEncoderV2; is still valid, but it is deprecated and has no effect. If you want to be explicit, please use pragma abicoder v2; instead.

https://docs.soliditylang.org/en/develop/layout-of-source-files.html#spdx-license-identifier

##Tools

Manual Review

##Recommended Mitigation
you can change it into `pragma abicoder v2;

  1. Title : missing amount check

the lock function was locked token that used for stakingReward it can be checked if amount can't be zero value

Tool Used

Manual Review

Recommended Mitigation

Check if amount > 0

  1. Title : example calculation comment was not same as actual code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/Aura.sol#L111

an e.g. was used to be

            // e.g. (new) reduction = (500 - 100) * 2.5 + 700 = 1700;

and the code was :

            uint256 reduction = totalCliffs.sub(cliff).mul(5).div(2).add(700);

since the code was mul first then div so it should be like implementation down below

             // e.g. (new) reduction = 500 - 100 * 5 / 2 + 700 = 1700;
          

this was used to be code mean, since calculation was remain the same. comment could be remain the same as it should be or it can be changed for good readibility.

Non Critical

  1. Title : Consider make the constracts pausable

There are many external risks so the suggestion was it should be consider making the contracts pausable, so if in the case of an unexpected event, the admin can pause transfers.

Tool Used

Manual Review

##POC
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

Recommended Mitigation Steps

Consider making contracts Pausable

  1. Title : Typo Comment

This typo comment It can be remain the same, or it can be changed instead for better readibility

Tool Used

Manual Review

POC

constructoor changed to constructor
ExtraRewardsDistributor.sol#L33
AuraBalRewardPool.sol#L55

Invdividual changed to individual
AuraLocker.sol#L22

  1. Title : Floating pragma

The contracts was used floating pragma ^0.8.11. It can be consider using locking the pragma version whenever possible and avoid using a floating pragma in the final deployment. Since it can be problematic, if there are publicly disclosed bugs and issues that affect the current compiler version used.

Tool Used

Manual Review

Recommendation

Use these compiler versions are to compile your code: 0.8.11, 0.8.12, 0.8.13, 0.8.14

  1. Title : Consistency for @author comment

Since AuraStakingProxy & AuraVestedEscrow was used to using @author adapted from ConvexFinance so ClaimFeesHelper inside contract should be remain the same as another

##Tool Used
Manual Review

@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels May 25, 2022
code423n4 added a commit that referenced this issue May 25, 2022
@0xMaharishi 0xMaharishi added the duplicate This issue or pull request already exists label May 28, 2022
@dmvt dmvt removed the duplicate This issue or pull request already exists label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

3 participants