Omniscia AmpleSense Audit

Distribute Static Analysis Findings

Distribute Static Analysis Findings

DIS-01S: Potential Unstake Re-Entrancy

Description:

The Distribute contract contains a re-entrancy whenever ETH rewards are pending for a particular user and an unstakeFrom operation is executed which can have wider ramifications for the contracts interfacing with the Distribute implementation.

Example:

contracts/Distribute.sol
52/**
53 @dev Stakes a certain amount, this MUST transfer the given amount from the caller
54 @param account Address who will own the stake afterwards
55 @param amount Amount to stake
56*/
57function stakeFor(address account, uint256 amount) public onlyOwner {
58 require(account != address(0), "Distribute: Invalid account");
59 require(amount > 0, "Distribute: Amount must be greater than zero");
60 _total_staked = _total_staked.add(amount);
61 if(_stakes[account] == 0) {
62 investor_count++;
63 }
64
65 uint256 accumulated_reward = getReward(account);
66 _stakes[account] = _stakes[account].add(amount);
67
68 uint256 new_bond_value = accumulated_reward * PRECISION / _stakes[account];
69 _bond_value_addr[account] = bond_value.sub(new_bond_value);
70}
71
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}
97
98 /**
99 @dev Withdraws rewards (basically unstake then restake)
100 @param account From whom
101 @param amount Amount to remove from the stake
102*/
103function withdrawFrom(address payable account, uint256 amount) external onlyOwner {
104 unstakeFrom(account, amount);
105 stakeFor(account, amount);
106}

Recommendation:

We strongly recommend the nonReentrant modifier security measure to be enforced on all sensitive functions of the Distribute contract to ensure that it is not possible to exploit the temporary unstaked units of a user.

Alleviation:

The nonReentrant modifier has been properly applied to all sensitive functions of the contract.