lemonmon - `msg_server_demand::FundTopic` passes incorrect additional fee, potentially activate a topic incorrectly

[sherlock-admin2]

lemonmon

Medium

msg_server_demand::FundTopic passes incorrect additional fee, potentially activate a topic incorrectly

Summary

msg_server_demand::FundTopic may activate a topic incorrectly, due to incorrect usage of the activateTopicIfWeightAtLeastGlobalMin function. The topic fee was updated before the activateTopicIfWeightAtLeastGlobalMin, but the added fee revenue (= msg.Amount) was passed as additional revenue, resulting in double counting of the added fund for the calculation of the topic weight.

Vulnerability Detail

When an user calls FundTopic to add a fee revenue for a topic, the calculation was done in the order of AddTopicFeeRevenue(ctx, msg.TopicId, msg.Amount) -> activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, msg.Amount):

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/msgserver/msg_server_demand.go#L44-L52

The AddTopicFeeRevenue updates the fee revenue by adding the amount in the parameter:

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/keeper.go#L1683-L1688

Then the activateTopicIfWeightAtLeastGlobalMin was called with the msg.Amount as the last parameter, which will be passed to the GetCurrentTopicWeight as the parameter additionalRevenue

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/msgserver/msg_server_util_topic_activation.go#L28-L36

In the GetCurrentTopicWeight, the topicFeeRevenue will be fetched using GetTopicFeeRevenue then the additionalRevenu will be added to get the newFeeRevenue, this value will be used as the fee revenue upon calculating the weight.

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/topic_weight.go#L61-L68

Note the the topic fee revenue fetched by the GetTopicFeeRevenue is already updated adding the msg.Amount in the FundTopic.

As the result the added fund msg.Amount will be added twice to calculate the weight. This might lead to activation of a topic, when they should not be activated if the weight is calculated correctly.

Impact

An incorrect activation of a topic

Code Snippet

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/msgserver/msg_server_demand.go#L44-L52

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/keeper.go#L1683-L1688

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/msgserver/msg_server_util_topic_activation.go#L28-L36

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/topic_weight.go#L61-L68

Tool used

Manual Review

Recommendation

Either change the call order to activateTopicIfWeightAtLeastGloblaMin -> AddTopicFeeRevenue, or change the parameter of the activateTopicIfWeightAtLeastGlolbalMin to alloraMath.ZeroDec().

Duplicate of #46

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Topic funding amount is incorrectly accounted for twice

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/allora-network/allora-chain/pull/505