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 #100

Open
code423n4 opened this issue May 19, 2022 · 1 comment
Open

QA Report #100

code423n4 opened this issue May 19, 2022 · 1 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 sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

openzeppelin initializable

description

Aura.sol uses the following check to make sure the initializer is only called once

/2022-05-aura/contracts/Aura.sol
67: require(totalSupply() == 0, "Only once");

however this is not best practice and there may be edge cases where this check fails

recommend inheriting from openzeppelin initializable instead

Modifier side-effects

description

Modifiers should only implement checks and not make state changes and external calls which violates the checks-effects-interactions pattern. These side-effects may go unnoticed by developers/auditors because the modifier code is typically far from the function implementation.

findings

/2022-05-aura/contracts/AuraBalRewardPool.sol
89:     modifier updateReward(address account) {
90:         rewardPerTokenStored = rewardPerToken();
91:         lastUpdateTime = lastTimeRewardApplicable();
92:         if (account != address(0)) {
93:             rewards[account] = earned(account);
94:             userRewardPerTokenPaid[account] = rewardPerTokenStored;
95:         }
96:         _;
97:     }
/2022-05-aura/contracts/AuraLocker.sol
170:     modifier updateReward(address _account) {
171:         {
172:             Balances storage userBalance = balances[_account];
173:             uint256 rewardTokensLength = rewardTokens.length;
174:             for (uint256 i = 0; i < rewardTokensLength; i++) {
175:                 address token = rewardTokens[i];
176:                 uint256 newRewardPerToken = _rewardPerToken(token);
177:                 rewardData[token].rewardPerTokenStored = newRewardPerToken.to96();
178:                 rewardData[token].lastUpdateTime = _lastTimeRewardApplicable(rewardData[token].periodFinish).to32();
179:                 if (_account != address(0)) {
180:                     userData[_account][token] = UserData({
181:                         rewardPerTokenPaid: newRewardPerToken.to128(),
182:                         rewards: _earned(_account, token, userBalance.locked).to128()
183:                     });
184:                 }
185:             }
186:         }
187:         _;
188:     }

rounding of penalty

description

if the reward is less than 5, the penalty will round down to zero

/2022-05-aura/contracts/AuraBalRewardPool.sol
183: uint256 penalty = (reward * 2) / 10;

recommend using openzeppelin's math util to round up

ceilDiv(uint256 a, uint256 b)

division before multiplication

description

due to rounding errors, multiplication should be done before any division

recommend to reorder the operations accordingly

findings

/2022-05-aura/contracts/CrvDepositorWrapper.sol
73: uint256 minOut = (((amount * 1e18) / bptOraclePrice) * minOutBps) / 10000;

Unspecific Compiler Version Pragma

description

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

findings

contracts in the code base use the follwing pragma

pragma solidity ^0.8.11;

no return statement

description

function is lacking a return statement when it should return bool

/2022-05-aura/convex-platform/contracts/contracts/BaseRewardPool.sol
314:     function donate(uint256 _amount) external returns(bool){
315:         IERC20(rewardToken).safeTransferFrom(msg.sender, address(this), _amount);
316:         queuedRewards = queuedRewards.add(_amount);
317:     }
@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 19, 2022
code423n4 added a commit that referenced this issue May 19, 2022
@0xMaharishi 0xMaharishi added invalid This doesn't seem right sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue labels May 25, 2022
@0xMaharishi
Copy link

These are mostly invalid findings in the context they are found in

@dmvt dmvt removed the invalid This doesn't seem right 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 sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants