sherlock-audit / 2024-06-allora-judging

0 stars 0 forks source link

lemonmon - `msg_server_registerations::Register` will overwrite reputerInfo, which can be used to sabotage other reputers #123

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 2 months ago

lemonmon

Medium

msg_server_registerations::Register will overwrite reputerInfo, which can be used to sabotage other reputers

Summary

Reputer information in the reputers Map can be overwritten. An adversary may abuse it to exclude reputers' report from the payloads by other honest reputer head nodes.

Vulnerability Detail

In the allora-inference-base's appchain::SendReputerModeData, the leader node will call the allora chain to get the other reputer's information via GetReputerAddressByP2PKey:

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-inference-base/cmd/node/appchain.go#L546

The Address of the resulting reputer info will be used to ignore multiple entries from the same address. Specifically, the node will include only the first entry per address. It means if multiple peers have the same allora address in the allora chain via GetReputerAddressByP2PKey, only first entry from the peers will be included by the leader node.

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-inference-base/cmd/node/appchain.go#L557

The GetReputerAddressByP2PKey from allora-chain returns the Owner address from the reputers Map:

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

On the allora-chain, one can register a reputer via Register transaction. It will eventually trigger msg_server_registrations::Register function.

In the Register function msg_register.Validate will be called and it will validate that the length of LibP2PKey.

After the registration fee is paid, InsertReputer will be called:

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

Which will update the topicReputers KeySet and reputers Map.

In the case that there is already an entry for the given LibP2PKey, it will overwrite the existing entry with the new information.

If somebody puts the same owner address for multiple LibP2PKey, only one of these peers will be included in the reputer payload.

Also, the Owner address does not need to be the Sender's address, so it can be set to any address.

Impact

An adversary can sabotage other reputers.

Code Snippet

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

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-inference-base/cmd/node/appchain.go#L546

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-inference-base/cmd/node/appchain.go#L557

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

Tool used

Manual Review

Recommendation

consider validating the LibP2PKey

Duplicate of #111

sherlock-admin4 commented 1 month ago

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

0xmystery commented:

Reputer and workers info can be overwritten via LibP2PKey

sherlock-admin2 commented 1 month ago

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