Omniscia AmpleSense Audit
Distribute Manual Review Findings
Distribute Manual Review Findings
DIS-01M: Deprecated Native Asset Transfer
Type | Severity | Location |
---|---|---|
Language Specific | Medium | Distribute.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:
72/**73 @dev unstakes a certain amounts, if unstaking is currently not possible the function MUST revert74 @param account From whom75 @param amount Amount to remove from the stake76*/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 too90 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
Type | Severity | Location |
---|---|---|
Logical Fault | Minor | Distribute.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:
108/**109 @dev Called contracts to distribute dividends110 Updates the bond value111 @param amount Amount of token to distribute112 @param from Address from which to take the token113*/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 pool124 _temp_pool = _temp_pool.add(amount);125 return;126 }127
128 // if a temp pool existed, add it to the current distribution129 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 bonds140 //it will be reinjected into the next distribution141 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.