Omniscia Tokemak Network Audit
BalanceTracker Manual Review Findings
BalanceTracker Manual Review Findings
BTR-01M: Improper Permitted Delegation
Type | Severity | Location |
---|---|---|
Logical Fault | Major | BalanceTracker.sol:L145-L150 |
Description:
The _delegate
function permits the delegation of units to an account whose balance has not been properly initialized. In such a case, an updateBalance
event on an account with a delegated amount will cause the amount to be overwritten as the userTokenBalance.token
value will be equal to the address(0)
.
Example:
91function updateBalance(92 address account,93 address token,94 uint256 amount,95 bool stateSync96) private {97 require(token != address(0), "INVALID_TOKEN_ADDRESS");98 require(supportedTokenAddresses.contains(token), "UNSUPPORTED_ADDRESS");99 require(account != address(0), "INVALID_ACCOUNT_ADDRESS");100
101 TokenBalance memory userTokenBalance = accountTokenBalances[account][token];102
103 // stateSync updates balances on an ongoing basis, whereas setBalance is only104 // allowed to update balances that have not been set before105 if (stateSync || userTokenBalance.token == address(0)) {106 //Update the total based on the individual amounts107 _updateTotalTokenBalance(token, userTokenBalance.amount, amount);108
109 accountTokenBalances[account][token] = TokenBalance({token: token, amount: amount});110 if (delegatedTo[account] != address(0)) {111 //Delegated balance, back out individual and apply new balance112 uint256 delegatedAmt = accountTokenBalances[delegatedTo[account]][token].amount;113 delegatedAmt = delegatedAmt.sub(userTokenBalance.amount).add(amount);114 accountTokenBalances[delegatedTo[account]][token] = TokenBalance({115 token: token,116 amount: delegatedAmt117 });118 }119
120 emit BalanceUpdate(account, token, amount, stateSync, true);121 } else {122 // setBalance may trigger this event if it tries to update the balance123 // of an already set user-token key124 emit BalanceUpdate(account, token, amount, false, false);125 }126}127
128function _delegate(129 address token,130 address from,131 address to132) private {133 require(from != address(0), "INVALID_FROM");134 require(token != address(0), "INVALID_TOKEN");135
136 TokenBalance memory balanceToTransfer = accountTokenBalances[from][token];137
138 //See if we need to back it out of an existing delegation139 if (delegatedTo[from] != address(0)) {140 TokenBalance memory oldDelegateBal = accountTokenBalances[delegatedTo[from]][token];141 oldDelegateBal.amount = oldDelegateBal.amount.sub(balanceToTransfer.amount);142 accountTokenBalances[delegatedTo[from]][token] = oldDelegateBal;143 }144
145 if (to != address(0)) {146 //Apply the existing balance to the new account147 TokenBalance memory newDelegateBal = accountTokenBalances[to][token];148 newDelegateBal.amount = newDelegateBal.amount.add(balanceToTransfer.amount);149 accountTokenBalances[to][token] = newDelegateBal;150 }151
152 delegatedTo[from] = to;153
154 emit BalanceDelegated(token, from, to);155}
Recommendation:
We advise a proper state transition to be enforced here whereby the recipient of the delegation has a non-zero token
member within their TokenBalance
struct.
Alleviation:
The code has been adjusted so that the _delegate
function also overwrites the token
entry of the newDelegateBal
, ensuring that it will always be non-zero.
BTR-02M: Inexplicable Function ID
Type | Severity | Location |
---|---|---|
Logical Fault | Informational | BalanceTracker.sol:L162 |
Description:
The function ID utilized for adjusting delegations is equal to "voting"
, however, it is not adequately justified why this conditional and validation exists given that the execution of _delegateAll
itself is conditional on the event type the contract receives.
Example:
157function _delegateAll(158 address from,159 address to,160 bytes32 functionId161) private {162 if (functionId == "voting") {163 uint256 length = supportedTokenAddresses.length();164 for (uint256 i = 0; i < length; i++) {165 address token = supportedTokenAddresses.at(i);166 _delegate(token, from, to);167 }168 }169}
Recommendation:
We advise this trait of the system to be re-evaluated and potentially adjusted as it can cause unintended no-ops in the _delegateAll
function.
Alleviation:
The Tokemak team has stated that this is desired behaviour and as such no remediation will be carried out for it.