sherlock-audit / 2024-06-allora-judging

0 stars 0 forks source link

0x416 - Lack of error handling when making blockless api call #131

Open sherlock-admin2 opened 4 months ago

sherlock-admin2 commented 4 months ago

0x416

Medium

Lack of error handling when making blockless api call

Summary

Lack of error handling when making blockless api call

Vulnerability Detail

Error handling when making blockless api call is missing.

Impact

In topics_handler.go, we are calling PrepareProposalHandler

func (th *TopicsHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
    return func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) {
        Logger(ctx).Debug("\n ---------------- TopicsHandler ------------------- \n")
        churnableTopics, err := th.emissionsKeeper.GetChurnableTopics(ctx)
        if err != nil {
            Logger(ctx).Error("Error getting max number of topics per block: " + err.Error())
            return nil, err
        }

        var wg sync.WaitGroup
        // Loop over and run epochs on topics whose inferences are demanded enough to be served
        // Within each loop, execute the inference and weight cadence checks and trigger the inference and weight generation
        for _, churnableTopicId := range churnableTopics {
            wg.Add(1)
            go func(topicId TopicId) {
                defer wg.Done()
                topic, err := th.emissionsKeeper.GetTopic(ctx, topicId)
                if err != nil {
                    Logger(ctx).Error("Error getting topic: " + err.Error())
                    return // @audit should contninue?
                }
    @           th.requestTopicWorkers(ctx, topic)
    @           th.requestTopicReputers(ctx, topic)
            }(churnableTopicId)
        }
        wg.Wait()
        // Return the transactions as they came
        return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil
    }

these two function requestTopicWorkers and requestTopicReputers trigger blockless api calls

for example,

the requestTopicReputers => go generateInferencesRequest => makeApiCall

err = makeApiCall(payloadStr)
    if err != nil {
        Logger(ctx).Warn(fmt.Sprintf("Error making API call: %s", err.Error()))
    }

the api call can fail for many reason, and if it fails, the code only log the error and does not return the error and bubble the error up for error handling.

then it means that the loss and inference request is never set out and updated,

the PrepareProposalHandler will still sliently assume the call always success and go through,

this leads to loss and inference data severely out of scope.

Code Snippet

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/app/app.go#L263

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/app/topics_handler.go#L152

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/app/topics_handler.go#L77

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/app/api.go#L160

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/app/api.go#L166

Tool used

Manual Review

Recommendation

handle the error from api call

sherlock-admin3 commented 4 months ago

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

0xmystery commented:

Error is not handled

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/458