volodya - anyone can rewrite reputer data and worker data

[sherlock-admin4]

volodya

High

anyone can rewrite reputer data and worker data

Summary

anyone can rewrite reputer data and worker data

Vulnerability Detail

Whenever users would like to register workers or reputer they call Register with msg.LibP2PKey that is visible to anyone on chain.

func (ms msgServer) Register(ctx context.Context, msg *types.MsgRegister) (*types.MsgRegisterResponse, error) {
    if err := msg.Validate(); err != nil {
        return nil, err
    }

    topicExists, err := ms.k.TopicExists(ctx, msg.TopicId)
    if err != nil {
        return nil, err
    }
    if !topicExists {
        return nil, types.ErrTopicDoesNotExist
    }

    hasEnoughBal, fee, err := ms.CheckBalanceForRegistration(ctx, msg.Sender)
    if err != nil {
        return nil, err
    }
    if !hasEnoughBal {
        return nil, types.ErrTopicRegistrantNotEnoughDenom
    }

    // Before creating topic, transfer fee amount from creator to ecosystem bucket
    err = ms.k.SendCoinsFromAccountToModule(ctx, msg.Sender, mintTypes.EcosystemModuleName, sdk.NewCoins(fee))
    if err != nil {
        return nil, err
    }

    nodeInfo := types.OffchainNode{
        NodeAddress:  msg.Sender,
        LibP2PKey:    msg.LibP2PKey, //@audit anyone can see it on blockchain
        MultiAddress: msg.MultiAddress,
        Owner:        msg.Owner,
        NodeId:       msg.Owner + "|" + msg.LibP2PKey,
    }

    if msg.IsReputer {
        err = ms.k.InsertReputer(ctx, msg.TopicId, msg.Sender, nodeInfo)
        if err != nil {
            return nil, err
        }
    } else {
        err = ms.k.InsertWorker(ctx, msg.TopicId, msg.Sender, nodeInfo)
        if err != nil {
            return nil, err
        }
    }

    return &types.MsgRegisterResponse{
        Success: true,
        Message: "Node successfully registered",
    }, nil
}

/msg_server_registrations.go#L43 There is a call to insert it into collection with reputerInfo.LibP2PKey, so anyone can rewrite with their own data. The same thing with InsertWorker

func (k *Keeper) InsertReputer(ctx context.Context, topicId TopicId, reputer ActorId, reputerInfo types.OffchainNode) error {
    topicKey := collections.Join(topicId, reputer)
    err := k.topicReputers.Set(ctx, topicKey)
    if err != nil {
        return err
    }
    err = k.reputers.Set(ctx, reputerInfo.LibP2PKey, reputerInfo) //@audit reputerInfo.LibP2PKey is a key so any other user can rewrite it 
    if err != nil {
        return err
    }
    return nil
}

emissions/keeper/keeper.go#L1432

Impact

reputer/worker data can be compromised

Code Snippet

Tool used

Manual Review

Recommendation

This seems to be safe

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

Duplicate of #111

[sherlock-admin3]

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