stonejiajia - Security Issues Report: Potential Panics in ABCI Methods

[sherlock-admin3]

stonejiajia

Medium

Security Issues Report: Potential Panics in ABCI Methods

Security Issues Report: Panics in ABCI Methods

Executive Summary

A security audit using CodeQL static analysis has revealed medium vulnerabilities in the ABCI (Application Blockchain Interface) methods of our Cosmos blockchain implementation. These vulnerabilities, primarily involving potential panics, pose a significant risk of causing chain halts. This report details the findings and provides recommendations for mitigation.

It is important to note that the criticality of panics in ABCI methods is well-documented in the Cosmos ecosystem. According to the official Cosmos security documentation (https://github.com/crytic/building-secure-contracts/blob/master/not-so-smart-contracts/cosmos/abci_panic/README.md), panics in ABCI methods are recognized as security issues that can lead to consensus failures and network-wide disruptions.

Findings

1. Panic in CosmosIntOneE18 Function

Severity: High

Location: common.go:14:3 https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/inference_synthesis/common.go#L11

Description: The CosmosIntOneE18 function contains a direct panic call, which could be triggered during the initialization of a large integer value.

Problematic Code:

func CosmosIntOneE18() cosmosMath.Int {
    ret, ok := cosmosMath.NewIntFromString("1000000000000000000")
    if !ok {
        panic("1*10^18 is not a valid cosmos int")
    }
    return ret
}

Risk: If this function is called within ABCI methods like BeginBlocker or EndBlocker, it could cause a chain halt if the integer initialization fails.

2. Potential Panics in EndBlocker Function

Severity: Medium

Location: Multiple locations within EndBlocker function, https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/module/abci.go#L14

Description: The EndBlocker function contains potential sources of panics:

EndBlocker -> EmitRewards -> GenerateRewardsDistributionByTopicParticipant -> GetRewardPerReputer -> GetRewardForReputerFromTotalReward -> CosmosIntOneE18 --> call to panic

Problematic Code Snippet:

func EndBlocker(ctx context.Context, am AppModule) error {
    // ... (code omitted for brevity)
    weights, sumWeight, totalRevenue, err := rewards.GetAndUpdateActiveTopicWeights(sdkCtx, am.keeper, blockHeight)
    if err != nil {
        return errors.Wrapf(err, "Weights error")
    }
    // ... (more code)
    err = rewards.EmitRewards(sdkCtx, am.keeper, blockHeight, weights, sumWeight, totalRevenue)
    if err != nil {
        sdkCtx.Logger().Error("Error calculating global emission per topic: ", err)
        return errors.Wrapf(err, "Rewards error")
    }
    // ...
}

Risk: Unhandled panics in this critical consensus method could lead to abrupt chain halts, potentially causing network-wide disruptions.

Recommendations

1. Refactor CosmosIntOneE18 Function:

   • Replace the panic with error returning mechanism.
   • Consider pre-computing this value as a constant to avoid runtime calculations.

   Example:

   var OneE18 cosmosMath.Int
   
   func init() {
      var ok bool
      OneE18, ok = cosmosMath.NewIntFromString("1000000000000000000")
      if !ok {
          log.Fatal("Failed to initialize OneE18")
      }
   }
   
   func CosmosIntOneE18() cosmosMath.Int {
      return OneE18
   }

2. Improve Error Handling in EndBlocker:

   • Implement proper error handling for all function calls that may return errors.
   • Add a recovery mechanism to catch and log potential panics without halting the chain.

3. Enhance Logging Practices:

   • Use structured logging to avoid potential panics from string formatting.

4. Implement Defensive Programming:

   • Add additional checks before critical operations (e.g., ensuring non-zero divisors).

5. Comprehensive Code Review:

   • Conduct a thorough review of all functions called within ABCI methods to ensure they do not contain hidden panics.

6. Regular Static Analysis:

   • Integrate CodeQL or similar static analysis tools into the CI/CD pipeline to catch potential panics early in the development process.

7. Error Recovery Mechanism:

   • Implement a wrapper function for EndBlocker to catch and handle any potential panics:

   func SafeEndBlocker(ctx context.Context, am AppModule) (err error) {
      defer func() {
          if r := recover(); r != nil {
              err = fmt.Errorf("recovered from panic in EndBlocker: %v", r)
              // Log detailed error information and stack trace
              // Attempt state recovery or rollback if possible
          }
      }()
   
      // Original EndBlocker logic
      // ...
   
      return nil
   }