Read the findings of the 2022 Web3 Security Report

`ARDImplementationV1` and `StakingToken` are upgradeable contracts, the admin can upgrade the contract without the community's commitment. If an attacker compromises the admin account, he can change the implementation of the contract and drain tokens from the contract.

Centralized Control of Contract Upgrade

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to different compiler versions. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Unlocked Compiler Version

In the contract `ARDImplementationV1` and `StakingToken` the contract deployer is granted with multiple privileged roles:

**DEFAULT_ADMIN_ROLE**

DEFAULT_ADMIN_ROLE has authority over the following functions:

- function revokeRole()
- function setAdminRole()
- function revokeAdminRole()
- function setMinterRole()
- function revokeMinterRole()
- function setBurnerRole()
- function revokeBurnerRole()
- function setAssetProtectionRole()
- function revokeAssetProtectionRole()
- function setSupplyControllerRole()
- function revokeSupplyControllerRole()


Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow the hacker to take advantage of this and grant mint/burn, pause/unpause, and freeze/unfreeze authorities to any externally owned address.

**MINTER_ROLE / BURNER_ROLE**

- Minter: allows minting ARD tokens
- Burner: allows burning ARD tokens

Any compromise to the above accounts may allow the hacker to take advantage of this authority and mint/burn ARD tokens to/from an arbitrary account, which could damage the whole tokenomics.

**ASSET_PROTECTION_ROLE**

- Asset Protection: allows doing any protection action about any account or asset. Can freeze,
unfreeze, and wipe any addresses.

**SUPPLY_CONTROLLER_ROLE**

SUPPLY_CONTROLLER_ROLE has authority over the following functions:

- function increaseSupply(): allows minting tokens to the supply controller account itself
- function decreaseSupply(): allows burning tokens from the supply controller account itself
- function setTokenBank()
- function enableStakingProtocol()
- function enableEarlyUnstaking()
- function setMinimumStake()
- function stakeFor()
- function unstakeFor()
- function setReward()
- function setRewardTable()
- function setPunishment()
- function setPunishmentTable()

Any compromise to the `SUPPLY_CONTROLLER_ROLE` account may allow the hacker to change settings in the staking protocol and mint arbitrary amounts of token to this account.

Centralization Related Risks

```solidity=
  function setAdminRole(address _addr) public {
        grantRole(DEFAULT_ADMIN_ROLE, _addr);
    }
    function revokeAdminRole(address _addr) public {
        revokeRole(DEFAULT_ADMIN_ROLE, _addr);
    }
```
The current implementation allows DEFAULT_ADMIN_ROLE to grant/revoke Role's admin role to specific account. By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means that only accounts with this role will be able to grant or revoke other roles. It should be noted that the `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to grant and revoke this role. Extra precautions should be taken to secure accounts that have been granted it. 

Any account that has `DEFAULT_ADMIN_ROLE` is able to revoke the `DEFAULT_ADMIN_ROLE` of the team and take over the control of the token contract.

DEFAULT_ADMIN_ROLE of the contract can be revoked by other authorized address

For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Otherwise it may be very difficult to write new implementation code. See https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable

Lack of storage gap in upgradeable contracts

In the current implementation, there is no function that tells user how much punishment they have to pay for the early unstake.

Missing function for punishment inquiry

In the current implementation of function `_unstake()`, the calculation of reward and punishment is based on the total value of a stake record. However, the current design allows users to unstake part of a stake multiple times and for each time, users will receive full reward/punishment that equals to the amount of unstaking the stake as a whole.

To better illustrate this issue, let's say user A stake 10000000000 ARD tokens in a 30-days-lock stake. He will receive 2% of the total staking amount which is 200000000 as rewards when he withdraws all of the stakes in one call after the lock expires. However, he can also choose to withdraw 100000000 ARD tokens each time and repeat 100 times. For every single call, he will receive 200000000 tokens as rewards and the total rewards here are 20000000000(100X of the intended reward amount).

The same will also happen when calculating the early unstake punishment. Users will receive much more punishment if they withdraw the stake multiple times.

Here is the vulnerable code:
```solidity=436
        if (stakingDays>=stakeholders[_stakeholder].stakes[stakeIndex].lockPeriod) {
            //Reward
            uint256 _reward = _calculateReward(_stakedAt, block.timestamp, 
                stakeholders[_stakeholder].stakes[stakeIndex].value,//Broken code !!!
                stakeholders[_stakeholder].stakes[stakeIndex].lockPeriod);
            if (_reward>0) {
                _mint(_stakeholder,_reward);
            }
            _transfer(address(this), _stakeholder, _value);
        } else {
            //Punishment
            require (earlyUnstakingAllowed, "early unstaking disabled");
            uint256 _punishment = _calculatePunishment(_stakedAt, block.timestamp, 
                stakeholders[_stakeholder].stakes[stakeIndex].value,
                stakeholders[_stakeholder].stakes[stakeIndex].lockPeriod);
            _punishment = _punishment<_value ? _punishment : _value;
            //If there is punishment, send them to tokenBank
            if (_punishment>0) {
                _transfer(address(this), tokenBank, _punishment); 
            }
            uint256 withdrawal = _value.sub( _punishment );
            if (withdrawal>0) {
                _transfer(address(this), _stakeholder, withdrawal);
            }
```

Critical logic issue that allows users to receive huge amounts of reward when they withdraw a single stake for multiple times

The computation of reward for each iteration can be very complex in some scenarios, especially when `stakes.length` is large. This could lead to OUT_OF_GAS exception.

```solidity=679
  function rewardOf(address _stakeholder)
        public
        view
        returns(uint256)
    {
        require(stakeholders[_stakeholder].totalStaked>0,"not stake holder");
        uint256 _totalRewards = 0;
        for (uint256 i = 0; i < stakeholders[_stakeholder].stakes.length; i++){
            Stake storage s = stakeholders[_stakeholder].stakes[i];
            uint256 r = _calculateReward(s.stakedAt, block.timestamp, s.value, s.lockPeriod);
            _totalRewards = _totalRewards.add(r);
        }
        return _totalRewards;
    }
```

Potential Gas Exhaustion

Using unit-test to test smart contracts is one of the best ways to identify potential logic errors and security vulnerabilities in the smart contract. Current text files found in the provided GitHub code repository are not comprehensive enough, based on the number of major/critical issues discovered in the audit.

Lack of Comprehensive unit-test

The current upper limit for punishment rate can be set to as high as 100%, which is not a reasonable value.

```solidity=
  function setPunishment(uint256 _lockPeriod, uint64 _value)
        public
        notPaused
        onlySupplyController
    {
        require(_value>=0 && _value<=10000, "invalid rate");
        uint256 ratesCount = punishmentTable[_lockPeriod].rates.length;
        uint256 oldRate = ratesCount>0 ? punishmentTable[_lockPeriod].rates[ratesCount-1].rate : 0;
        require(_value!=oldRate, "same as it is");
        punishmentTable[_lockPeriod].rates.push(Rate({
            timestamp: block.timestamp,
            rate: _value
        }));
        emit PunishmentRateChanged(block.timestamp,_value,oldRate);
    }
```

Missing proper upper limit for punishment rate

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Redundant Code Components

Ardis

ARDImplementationV1.sol

Ardis

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

11

0

0

2

9