QA Report #287

The comment for approveRewardDistributor() clearly states that the mentioned function should modify approvals for a distributor, ie revoke or give permission to an address to distribute rewards but the function name demonstrates a one sided role.Therefore change the name to reflect the function's dual role.

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

Also, consider emitting an event to alert relevant parties when a distributor is approved or disapproved.

Check whether the old address is the same as the new address

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

cache old address in a local variable and either use :

if(oldDao≠_newDao) {

dao=_newDao;

}

Apply the same changes to the following contracts with modification to variable names for the aforementioned functions :

AuraMerkleDrop.sol

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

AuraStakingProxy.sol

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

and consider emitting an event when the crvDepositorWrapper is changed.

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

also, it is recommended to emit an event for this function as well.

AuraVestedEscrow.sol

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

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

Array Length should match

If there's a missing amount for a recipient address, the mapping will not contain the amount attributable to the address nor will it be added to the totalAmount :

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L103-L104

And therefore less will be sent to the AuraVestedEscrow contract if there's a non-zero value but it was not added to the array:

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

Use require(_recipients.length==_amount.length,”mismatch”) prior to the loop.

Similar case for distribute ()

ArbitratorVault.sol

[https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/ArbitartorVault.sol#L46

Crv balance locked for staker can be unlocked sooner

Due to an incorrect value set :
https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/CrvDepositor.sol#L26

The balance will be unlocked a day earlier than it should actually be. MAXTIME is calculated using 364 days, instead of 365 days which is standard for a year.

This value is used to calculate unlock At which is subsequently stored in the state variable unlockTime.
initialiseLock() and lockCurve() relies on this value. It would be imperative that the unlock time is calculated properly, as the intent is for the lock period to be a year.

A similar finding was mentioned here

code-423n4/2022-03-paladin-findings#4

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

Hawkeye issue #287

99dacdb

0xMaharishi added the duplicate This issue or pull request already exists label May 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QA Report #287

QA Report #287

code423n4 commented May 25, 2022

QA Report #287

QA Report #287

Comments

code423n4 commented May 25, 2022

QA

Grammatical Errors

NatSpec tag missing

Constants should follow style guide

Use mixedCase for Immutable state variables

State Variables are already their default types

Comment doesn't reflect the code

Function names should be unambiguous

Array Length should match

Crv balance locked for staker can be unlocked sooner