Omniscia AmpleSense Audit

Distribute Manual Review Findings

Distribute Manual Review Findings

DIS-01M: Deprecated Native Asset Transfer

TypeSeverityLocation
Language SpecificMediumDistribute.sol:L94

Description:

The transfer member accessible in payable variables forwards a fixed gas stipend for the operation which can change on upcoming EIPs as evidenced by f.e. EIP-2929.

Example:

contracts/Distribute.sol
72/**
73 @dev unstakes a certain amounts, if unstaking is currently not possible the function MUST revert
74 @param account From whom
75 @param amount Amount to remove from the stake
76*/
77function unstakeFrom(address payable account, uint256 amount) public onlyOwner {
78 require(account != address(0), "Distribute: Invalid account");
79 require(amount > 0, "Distribute: Amount must be greater than zero");
80 require(amount <= _stakes[account], "Distribute: Dont have enough staked");
81 uint256 to_reward = _getReward(account, amount);
82 _total_staked = _total_staked.sub(amount);
83 _stakes[account] = _stakes[account].sub(amount);
84 if(_stakes[account] == 0) {
85 investor_count--;
86 }
87
88 if(to_reward == 0) return;
89 //take into account dust error during payment too
90 if(address(reward_token) != address(0)) {
91 reward_token.safeTransfer(account, to_reward);
92 }
93 else {
94 account.transfer(to_reward);
95 }
96}

Recommendation:

We strongly recommend a more robust payout method to be utilized such as the sendValue function of OpenZeppelin's Address library that forwards the currently available gas and is guaranteed to always be executable.

Alleviation:

The sendValue method of the OpenZeppelin Address library is now properly utilized.

DIS-02M: Inexistent Prohibition of Improper Invocation

TypeSeverityLocation
Logical FaultMinorDistribute.sol:L115-L118

Description:

The distribute function is meant to act for both native asset and ERC20 asset distributions, however, in doing so it can lead to native asset funds being permanently locked if both the native asset and a non-zero amount are specified when reward_token has been defined.

Example:

contracts/Distribute.sol
108/**
109 @dev Called contracts to distribute dividends
110 Updates the bond value
111 @param amount Amount of token to distribute
112 @param from Address from which to take the token
113*/
114function distribute(uint256 amount, address from) external payable onlyOwner {
115 if(address(reward_token) != address(0)) {
116 if(amount == 0) return;
117 reward_token.safeTransferFrom(from, address(this), amount);
118 } else {
119 amount = msg.value;
120 }
121
122 if(_total_staked == 0) {
123 // no stakes yet, put into temp pool
124 _temp_pool = _temp_pool.add(amount);
125 return;
126 }
127
128 // if a temp pool existed, add it to the current distribution
129 if(_temp_pool > 0) {
130 amount = amount.add(_temp_pool);
131 _temp_pool = 0;
132 }
133
134 uint256 temp_to_distribute = to_distribute.add(amount);
135 uint256 total_bonds = _total_staked / PRECISION;
136 uint256 bond_increase = temp_to_distribute / total_bonds;
137 uint256 distributed_total = total_bonds.mul(bond_increase);
138 bond_value = bond_value.add(bond_increase);
139 //collect the dust because of the PRECISION used for bonds
140 //it will be reinjected into the next distribution
141 to_distribute = temp_to_distribute.sub(distributed_total);
142}

Recommendation:

We advise the AmpleSense team to introduce a require check that ensures msg.value is equal to 0 in the first if clause to guarantee no native assets are ever locked in the contract.

Alleviation:

The require check we advised has been introduced to the codebase preventing lock of native assets.