carrotsmuggler - Anyone can overwrite reputer `p2pkey` values

[sherlock-admin4]

carrotsmuggler

High

Anyone can overwrite reputer p2pkey values

Summary

Anyone can overwrite reputer p2pkey values

Vulnerability Detail

The Register function in the msg_server_registrations.go file is used to register new reputers or workers.

This function also takes in a LibP2PKey value to identify the reputer/worker.

nodeInfo := types.OffchainNode{
    NodeAddress:  msg.Sender,
    LibP2PKey:    msg.LibP2PKey,
    MultiAddress: msg.MultiAddress,
    Owner:        msg.Owner,
    NodeId:       msg.Owner + "|" + msg.LibP2PKey,
}

//@audit anyone can add these as long as they pay the fund. can overwrite other user p2p keys
if msg.IsReputer {
    err = ms.k.InsertReputer(ctx, msg.TopicId, msg.Sender, nodeInfo)
    if err != nil {
        return nil, err
}

However, the issue is that this LibP2PKey value is not checked. This value is actually used when inserting the reputer, and is used to set the reputers mapping in the keeper.

err = k.reputers.Set(ctx, reputerInfo.LibP2PKey, reputerInfo)

So a user can pass in some other reputer's LibP2PKey value, and it will overwrite this mapping and assign to the new reputer using this value. Thus the LibP2PKey value will now point to the new reputer's nodeInfo and thus its address instead of the original reputer's values.

The main issue is that the protocol uses this mapping in its internals. The GetReputerByLibp2pKey function is called by the GetReputerNodeInfo function in the query_server_registrations.go file, and this function is used throughout the SDK to get reputer addresses.

So users can hijack a libp2pkey and force the system to address it whenever using that key.

Impact

This allows any user to hijack a libp2pkey being used by another reputer/worker. Users using GetReputerByLibp2pKey to find reputer addresses will also be redirected to the wrong reputer address.

Code Snippet

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

Tool used

Manual Review

Recommendation

Disallow overwriting of LibP2PKey values. If the LibP2PKey value exists, revert during registration.

Duplicate of #111

[sherlock-admin4]

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]

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