lemonmon - `msg_server_stake::AddStake` calculates the weight incorrectly resulting in incorrect activation of a topic

[sherlock-admin4]

lemonmon

Medium

msg_server_stake::AddStake calculates the weight incorrectly resulting in incorrect activation of a topic

Summary

Allora calculates the topic's weight based on the stake amount and fee revenue. In the msg_server_stake::AddStake function, the activateTopicIfWeightAtLeastGlobalMin was used incorrectly. It may activate the topic incorrectly.

Vulnerability Detail

When a reputer send transaction to AddStake, at the end of the function, activateTopicIfWeightAtLeastGlobalMin will be called:

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

The function will calculate the weight based on updated stake for the reputer and if the weight is large enough, the topic will be activated.

Note that the last input to the call was msg.Amount which is the added stake by the reputer.

In the activateTopicIfWeightAtLeastGlobalMin will call GetCurrentTopicWeight to calcaulate the new weight for the topic, and use the new weight to determine whether the topic should be activated:

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#L41-L49

The last input the the activateTopicIfWeightAtLeastGlobalMin will be passed to GetCurrentTopicWeight as the last parameter. In the GetCurrentTopicWeight uses the last parameter additionalRevenue as the added topic fee revenue. It will be added to the existing topic fee revenue and passed to the GetTargetWeight as the fee revenue.

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

The topic's stake amount will be fetched using GetTopicStake:

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

The topic's stake amount is correct, since the topic's stake was already updated in the AddStake function.

The GetTargetWeight will calculate topic weight using both stake of the topic and fee revenue.

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

// Return the target weight of a topic
// ^w_{t,i} = S^{μ}_{t,i} * (P/C)^{ν}_{t,i}
// where S_{t,i} is the stake of of topic t in the last reward epoch i
// and (P/C)_{t,i} is the fee revenue collected for performing inference per topic epoch
// requests for topic t in the last reward epoch i
// μ, ν are global constants with fiduciary values of 0.5 and 0.5

As the result the added amount of stake will be considered as the fee revenue, and the weight will be calculated accordingly.

Impact

When a reputer adds stake, it will calculate the topic's weight incorrectly, resulting in incorrect activation of the topic.

Code Snippet

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

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#L41-L49

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

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

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

Tool used

Manual Review

Recommendation

use zero in the place of added fee revenue.

// https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/msgserver/msg_server_stake.go#L54
-  err = activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, msg.Amount)
+  err = activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, alloraMath.ZeroDec())

[sherlock-admin3]

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

0xmystery commented:

Topic funding amount is incorrectly accounted for twice

[relyt29]

I think this is a separate issue not a duplicate, as mentioned in #77

[0x3b33]

Escalate

The same issue can be seen inside AddStake, where in this case, the stake amount is accidentally added as topic revenue.

I would consider this worthy of its own separate bug

As mentioned by the sponsor in this comment this is issue can be it's own separate bug as it mentions how AddStake amount is accidentally added as topic revenue, causing topics to be activated even if they haven't reached the required weight.

77 then would be a duplicate of this.

[sherlock-admin3]

Escalate

The same issue can be seen inside AddStake, where in this case, the stake amount is accidentally added as topic revenue.

I would consider this worthy of its own separate bug

As mentioned by the sponsor in this comment this is issue can be it's own separate bug as it mentions how AddStake amount is accidentally added as topic revenue, causing topics to be activated even if they haven't reached the required weight.

77 then would be a duplicate of this.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[mystery0x]

Escalate

The same issue can be seen inside AddStake, where in this case, the stake amount is accidentally added as topic revenue.

I would consider this worthy of its own separate bug

As mentioned by the sponsor in this comment this is issue can be it's own separate bug as it mentions how AddStake amount is accidentally added as topic revenue, causing topics to be activated even if they haven't reached the required weight.

77 then would be a duplicate of this.

I agree that this should likely be a separate bug.

[WangSecurity]

I agree both are different issues, even though they seem the same (adding more value than it should). But in #46 it double adds to the fee revenue, while in this report it adds the staked amount to the fee revenue.

Hence, planning to accept the escalation, and make a new family with medium severity. This issue will be the best, #77 will be the duplicate.

[WangSecurity]

Result: Medium Has duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0x3b33: accepted

[sherlock-admin2]

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