Sign Up for a Code Audit Today and Get 3 Months of Skynet for Free

In contract `ArgoStaking`, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `ArgoStaking.setxArgo`: Set address of xARGO token
- `ArgoStaking.setxArgoPledging`: Set address of xArgoPledging contract
- `AccessControlUpgradeable.grantRole`: Gives an address a privileged role
- `AccessControlUpgradeable.revokeRole`: Removes a privileged role from an address
- `AccessControlUpgradeable.renounceRole`: removes the function caller's `DEFAULT_ADMIN_ROLE` role

Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and change the address of xARGO token and xArgoPledging contract.

In contract `xArgo`, the address stored in `argoStaking` has the authority over the following functions:
- `xArgo.mintTo()`: Mint a designated amount of xArgo tokens to a designated address
- `xArgo.burn()`: Burn a designated amount of xArgo tokens owned by the address stored in `argoStaking`

Any compromise to the `argoStaking` account may allow a hacker to take advantage of this authority and change the supply of xArgo tokens.


In contract `xArgoPledging`, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `xArgoPledging.addReward`: Add a new reward token to be distributed to stakers
- `xArgoPledging.approveRewardDistributor`: Modify approval for an address to call notifyRewardAmount
- `xArgoPledging.setMORALEStartTime`: Modify start time for MORALE farming
- `xArgoPledging.setMORALERate`: Modify rate of MORALE farming
- `xArgoPledging.modifyEmissions`: Function to restart emissions if required
- `AccessControlUpgradeable.grantRole`: Gives an address a privileged role
- `AccessControlUpgradeable.revokeRole`: Removes a privileged role from an address
- `AccessControlUpgradeable.renounceRole`: removes the function caller's `DEFAULT_ADMIN_ROLE` role


Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and modify start time and farming rate.

Centralization Related Risks in Contract `ArgoStaking`

`SafeERC20Upgradeable` library is declared and attached to `IERC20Upgradeable`. However the underlying safe functions are not used and instead regular `transfer` or `transferFrom` are used.

Safe functions are unused

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks.

Incompatibility With Deflationary Tokens

Function that affects the status of sensitive variables should emit events as notifications.
- `_modifyEmissions`: Changes the variables used for checking requirements and calculating rewards
- `updatePool`: Update reward variables of the pool

Missing Emit Events

Duplicate Comments

The return value from `call` is not properly handled. 
```solidity=309
  payable(msg.sender).call{value: reward}("");
```
If the return value of a low-level message call is not checked, the execution will resume even if the called contract throws an exception. Even though the transfer failed, the function will not be reverted automatically. Therefore, it could lead to users' loss of rewards if there is not enough `CRO` in the contract.



Unhandled Value of Low-level Call

Although the contract inherits the contract `PausableUpgradeable`, there are no functions implementing the functions to pause the contract. Therefore the contract cannot be paused and the modifier is rendered useless. 

Contract can not be paused

The comment suggest that the condition checks `If account address is not ArgoStaking or controller`. However the actual condition checks `if (account != address(0))`.

Misleading Comments

The function that affects the status of sensitive variables should emit events as notifications.
- `xArgoPledging.pledge`: Initiate a stake
- `xArgoPledging.unpledge`: Execute an unstake

`SafeERC20Upgradeable` library is declared and attached to `IERC20Upgradeable`. However the underlying safe function are not used and instead regular `transfer` is used.

The `xArgoPledging` protocol maintains two set of logic for reward, which work separately:
1. The reward logic inherits from `xArgoFarm` contract, in which the rewards are distributed during `deposit` and `withdraw`. In this logic, the source of reward token is unknown.
2. The reward logic implemented in side the `xArgoPledging`. In this logic, the `notifyRewardAmount()` will transfer tokens to the current contract as reward and update the reward rate at the same time.

Therefore, in the first reward logic, the source of the reward token is unknown. If both reward logic use `xArgo` as reward and there are not enough tokens within the contract, it could lead to unexpected behavior in both reward logic.

Discussion on Token Flow

The following function that affects the status of sensitive variables should emit events as notifications.
- `ArgoStaking.setxARGO`: Set address of xARGO token
- `ArgoStaking.setxArgoPledging`: Set address of xArgoPledging contract

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks.

In the function `ArgoStaking.stakeARGO()`, the `ARGO` token initialized in the `__ArgoStaking_init()` function. If the `ARGO` token is a deflationary token, it could lead to unexpected loss to the project.

Argo - Audit - Addendum

ArgoStaking.sol

IWrapped.sol

IMintBurnToken.sol

IArgoRewards.sol

xArgoFarm.sol

xArgo.sol

xArgoPledging.sol

All of the Argo tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute Argo tokens without obtaining the consensus of the community.

Initial Token Distribution

The roles owner and `DEFAULT_ADMIN_ROLE` are unused in the contract.

Unused Privileged Role

In contract `ArgoImpl`, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `ArgoImpl.setbCRO`: Sets a new bCRO Token deployed address
- `ArgoImpl.withdraw`: Withdraw CRO from contract
- `ArgoImpl.setTreasury`: Sets a new treasury address
- `ArgoImpl.setBridgeWallet`: Sets a new Crypto.org Chain wallet address
- `ArgoImpl.setStakeFeeRate`: Sets new stake fee rate
- `ArgoImpl.setUnstakeFeeRate`: Set fee rate for unstaking
- `AccessControlUpgradeable.grantRole`: Gives an address a privileged role
- `AccessControlUpgradeable.revokeRole`: Removes a privileged role from an address
- `AccessControlUpgradeable.renounceRole`: removes the function caller's `DEFAULT_ADMIN_ROLE` role
- `ArgoImpl.pauseContract`: Set contract status to pause
- `ArgoImpl.unpause`: Set contract staus to unpause

In addition, the role `CONTROLLER_ROLE` has the authority over the following functions:
- `setExchangeRate`: Sets bCRO-CRO exchange rate
- `getBridgeWallet`: get bridge wallet address

Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and change fees or update the treasury. Any compromise to `CONTROLLER_ROLE` account may allow a hacker to change the exchange rate of bCRO and CRO.

Centralization Related Risks in Contract `ArgoImpl`

The contract `ArgoImpl` inherited from contract `PausableUpgradeable` with modifier `whenNotPaused` on functionalities, for example, `stake`, `unstake`, `claimCRO` and `setExchangeRate`.
The `pause` is initiated via `__Pausable_init_unchained();` during initialization and the value is `false`.
There is no further functionality to change the pause in the rest of the codebase.
The functions mentioned above cannot be paused. 

Unable to Pause Contract

`bCROToCROExchangeRate` is used as bCRO-CRO exchange rate to mint or burn `bCRO` token during staking and unstaking. The value of `bCROToCROExchangeRate` can be updated with function `setExchangeRate()`.
The concern is, that there is no time limitation on the function `setExchangeRate()`. It can be called by the `CONTROLLER` at any time which may cause insufficient funds to claim.

For example,
when a user stakes 10 CRO tokens at the initial `bCROToCROExchangeRate`, the `bCROAmount` will be 10 * 1 / 1 = 10. 
However if `bCROToCROExchangeRate` is increased to 2 at the time of staking, the `croAmount` will be 10 * 2 / 1 = 20. The contract will have insufficient funds and revert when `claimCRO` function is called.



Discussion on `bCROToCROExchangeRate` Change

Although `SafeERC20Upgradeable` is attached to `IERC20Upgradeable`, the functions are not used.

`SafeERC20Upgradeable` is declared but safe functions are not used

Unused Mapping

The comment states `Can only be called by admin`, however the function does not have any access control. Furthermore, even if the function implements access control, private variables can still be read from the blockchain.

Inconsistent Comment and Code

When user stake to the `ArgoImpl` contract, the `CRO` will be sent to bridge address with `croBridge.send_cro_to_crypto_org{value: stakeAmount}(CDO_ADDRESS);`.
When user claim `CRO`, the `CRO` will be sent from `ArogImpl` contract via ` payable(msg.sender).call{value: unstakeAmount}("");`.

The concern is, that the current money(`CRO`) flow is not consistent because based on the current implementation in the `AgoImpl` contract it should not hold `CRO` during staking but should maintain `CRO` when the user claiming. 
There is a lack of introduction about how `CRO` moves between `ArgoImpl` contract and bridge.



In contract `bCRO`, `argoImpl` address has the authority over the following functions:
- `bCRO.mintTo`: Mint designated the number of `bCRO` to the specified address 
- `bCRO.burn`: Burn designated number of `bCRO` tokens from `argoImpl` account

Any compromise to `argoImpl` account may allow a hacker to take advantage of this authority and mint desired amount of tokens.


Centralization Related Risks in Contract `bCRO`

The following functions affect the status of sensitive variables and should be able to emit events as notifications:
- `bCRO.burn`

In contract `TokenVaultBase`, the owner has the authority over the following functions:
- `TokenVaultBase.withdraw`: Transfer tokens owned by `TokenVaultBase` contract
- `Ownable.renounceOwnership`: Remove owner from contract
- `Ownable.transferOwnership`: Change owner of contract

Since the contracts `CommunityBountyVault`, `IncentiveProgramsVault`, `MarketingVault`, `TeamVault` and `IncentiveProgramsVault`(Treasury Vault) are inherited from the contract `TokenVaultBase`, the owner for each conract have privilege on aforemention functions in the corresponding contract. 

Any compromise to the owner accounts may allow a hacker to take advantage of this authority and withdraw Argo tokens owned by the corresponding contract.

Centralization Related Risks in `TokenVaultBase`  Contracts

The filename is `TreasuryVault.sol`, however the contract name is `IncentiveProgramsVault`, which a duplicate contract already exists under the file `IncentiveProgramsVault`.

Different Contract Name and File Name

Despite the file name being `TreasuryVault`, the NatSpec states the contract title as `IncentiveProgramsVault` and the purpose of the contract as to hold tokens for the incentive program.

Argo

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

14

0

4

0

10