Description

The Voter contract collects fees from the lending pools and distributes fees according to the share of each user. The share is decided by the voting weight of the lock that the user voted to the lending pools through.

In order to track the collected fees, the Voter contract has the tokenLastBalance array variable, which stores the amount of tokens recorded at the last checkpoint logic. When the checkpoint logic runs, it calculates how many tokens are added compared to the last checkpoint logic, adds it to the collected fees of the term, and updates the tokenLastBalance variable.

The minter, the privileged account in the Voter contract, can add the token on the contract, suspend the distribution of the token, and resume the distribution of the token. When the distribution of the token is suspended, its value in tokenLastBalance is erased by shifting all elements after the element of the token to be suspended:

function suspendToken(address _token) external onlyMinter {
	// ...
	suspendedTokenLastBalance[_token] = tokenLastBalance[arrIdx]; // save current tokenLastBalance to suspendedTokenLastBalance
	for (uint256 i = arrIdx; i < tokens.length - 1; i++) {
		address iToken = tokens[i + 1];
		tokens[i] = iToken;
		tokenIndex[iToken] = tokenIndex[iToken] - 1;
		uint256 nextTLastBalance = tokenLastBalance[i + 1];
		tokenLastBalance[i] = nextTLastBalance;
	}

	tokens.pop();
	tokenIndex[_token] = 0;
	pools[_token] = address(0);
	isSuspended[_token] = true;
}

However, this leaves the last element of tokenLastBalance undeleted. For instance, if the tokenLastBalance array is $[1,2,3,0,0,\dots ]$ and the minter suspends the first token, the tokenLastBalance array changes to $[2,3,3,0,0,\dots ]$ instead of $[2,3,0,0,0,\dots ]$.

Impact

This bug has two outcomes. First, it will prevent the minter from suspending another token, because the suspendToken checks if the next of the last element of tokenLastBalance is not zero:

function suspendToken(address _token) external onlyMinter {
	// ...
	uint256 vacantTokenLastBalance = tokenLastBalance[tokens.length];
	require(
		vacantTokenLastBalance == 0,
		"unexpected error: tokenLastBalance without token is greater than 0"
	);
	// ...
}

A new token should be added to the Voter contract to suspend another token.

Second, the collected fees will not be distributed as expected. More specifically, if the balance of the token added after suspension is lower than the last element value of the tokenLastBalance, the entire token-distribution logic will revert and not work. If not, the token-distribution logic will not revert, but the part of the collected token that was added after suspension will be locked as much as the last element value of the tokenLastBalance.

Recommendations

Consider erasing the last element of the tokenLastBalance array in the token-suspension logic.

Remediation

This issue has been acknowledged by Familia Labs Ltd., and a fix was implemented in commit 1cb86f64↗.