sherlock-audit / 2024-06-allora-judging

0 stars 0 forks source link

pks_ - Unlimited topic parameters size when creating topic can cause node DoS #100

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

pks_

High

Unlimited topic parameters size when creating topic can cause node DoS

Summary

Create topic action don't limit topic.Metadata and topic.DefaultArg parameters size may cause node DoS by increasing large log file.

Vulnerability Detail

When creating topic by CreateTopic action, there is no any limit on topic.Metadata and topic.DefaultArg parameters size inside msg.Validate function:

func (msg *MsgCreateNewTopic) Validate() error {
    ...
    if len(msg.DefaultArg) == 0 {
        return errors.Wrap(sdkerrors.ErrInvalidRequest, "default argument cannot be empty")
    }
    ...
}

which means any users can create topic with large topic.Metadata and topic.DefaultArg parameters size. When process the txs, the chain scan all block txs and process in abci.go#EndBlocker function of emissions module and log the id, metadata and defaultArgs parameters into log file:

func EndBlocker(ctx context.Context, am AppModule) error {
    ...
    if am.keeper.CheckCadence(blockHeight, topic) {
        sdkCtx.Logger().Debug(fmt.Sprintf("ABCI EndBlocker: Inference cadence met for topic: %v metadata: %s default arg: %s. \n",
            topic.Id,
            topic.Metadata,
            topic.DefaultArg))
    ...
}

This cause one issue may cause node DoS: malicious users can create topic with large topic.Metadata and topic.DefaultArg parameters size by specify target chain node, then DelegateStake and RemoveDelegateStake with 1 wei as delegator, both the two functions have no amount size:

func (ms msgServer) DelegateStake(ctx context.Context, msg *types.MsgDelegateStake) (*types.MsgDelegateStakeResponse, error) {
    if msg.Amount.IsZero() {
        return nil, types.ErrReceivedZeroAmount
    }
    ...
}

func (ms msgServer) RemoveDelegateStake(ctx context.Context, msg *types.MsgRemoveDelegateStake) (*types.MsgRemoveDelegateStakeResponse, error) {
    if msg.Amount.IsZero() {
        return nil, types.ErrReceivedZeroAmount
    }
    ...
}

so the cost is very low. Malicious users can repeat such attack crossing many blocks to increasing the node log file. The node may DoS if the log file is large enough.

Impact

Create topic action don't limit topic.Metadata and topic.DefaultArg parameters size may DoS node with large log file.

Code Snippet

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/module/abci.go#L57-L60

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

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/types/msg_create_topic.go#L10

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

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

Tool used

vscode, Manual Review

Recommendation

Add strict parameters size validate inside topic validate function.

relyt29 commented 4 months ago

Topic creation costs money, it has a fee, which will be tuned to prevent spam transactions

furthermore, each block has a block size cap so the log DoS attack proposed would require money to pull off and would be extremely obvious as you'd have to submit many many transactions with full blocks to have the logs fill.

That said, adding a limit for these parameters is not difficult so we do it here: https://github.com/allora-network/allora-chain/pull/468

sherlock-admin2 commented 4 months ago

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

mystery0x commented 4 months ago

DoS not possible as topic creation costs money