Omniscia Tokemak Network Audit

BalanceTracker Manual Review Findings

BalanceTracker Manual Review Findings

BTR-01M: Improper Permitted Delegation

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:

contracts/balance-tracker/BalanceTracker.sol
91function updateBalance(
92 address account,
93 address token,
94 uint256 amount,
95 bool stateSync
96) 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 only
104 // allowed to update balances that have not been set before
105 if (stateSync || userTokenBalance.token == address(0)) {
106 //Update the total based on the individual amounts
107 _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 balance
112 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: delegatedAmt
117 });
118 }
119
120 emit BalanceUpdate(account, token, amount, stateSync, true);
121 } else {
122 // setBalance may trigger this event if it tries to update the balance
123 // of an already set user-token key
124 emit BalanceUpdate(account, token, amount, false, false);
125 }
126}
127
128function _delegate(
129 address token,
130 address from,
131 address to
132) 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 delegation
139 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 account
147 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

TypeSeverityLocation
Logical FaultInformationalBalanceTracker.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:

contracts/balance-tracker/BalanceTracker.sol
157function _delegateAll(
158 address from,
159 address to,
160 bytes32 functionId
161) 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.