Skip to content

fix: prevent gas type overflow in ABCI responses #25271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

fix: prevent gas type overflow in ABCI responses #25271

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8b6626a
Update abci.go
DeVikingMark Aug 27, 2025
8b0016a
Update abci.go
DeVikingMark Aug 27, 2025
e025e7e
Update abci_test.go
DeVikingMark Aug 27, 2025
beccde3
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
DeVikingMark Aug 30, 2025
bc343b5
Update int.go
DeVikingMark Aug 30, 2025
4991c0c
Update int_test.go
DeVikingMark Aug 30, 2025
d7df113
Update abci.go
DeVikingMark Aug 30, 2025
3621fe2
Update baseapp.go
DeVikingMark Aug 30, 2025
ce45330
Update abci.go
DeVikingMark Aug 30, 2025
f571554
Update abci_test.go
DeVikingMark Aug 30, 2025
0dd9e93
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
aljo242 Sep 2, 2025
5cffdf1
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
aljo242 Sep 6, 2025
53d4506
Update deterministic_test.go
DeVikingMark Sep 16, 2025
df07616
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
DeVikingMark Sep 16, 2025
1ec6aec
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
aljo242 Oct 23, 2025
b305509
test for codecov
DeVikingMark Oct 23, 2025
15c38b9
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
aljo242 Oct 24, 2025
7a76547
Merge branch 'main' into prevent-gas-type-overflow-in-ABCI-responses
aljo242 Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions baseapp/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import (
"context"
"fmt"

Check failure on line 5 in baseapp/abci.go

View workflow job for this annotation

GitHub Actions / Analyze 

File is not properly formatted (gci)

Check failure on line 5 in baseapp/abci.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

File is not properly formatted (gci)
"sort"
"strings"
"time"
Expand All @@ -20,6 +20,8 @@
snapshottypes "cosmossdk.io/store/snapshots/types"
storetypes "cosmossdk.io/store/types"

"math"

"github.com/cosmos/cosmos-sdk/baseapp/state"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/telemetry"
Expand Down Expand Up @@ -334,6 +336,15 @@
}
}

// safeInt64FromUint64 converts uint64 to int64 with overflow checking.
// If the value is too large to fit in int64, it returns math.MaxInt64.
func safeInt64FromUint64(val uint64) int64 {
if val > math.MaxInt64 {
return math.MaxInt64
}
return int64(val)
}

// CheckTx implements the ABCI interface and executes a tx in CheckTx mode. In
// CheckTx mode, messages are not executed. This means messages are only validated
// and only the AnteHandler is executed. State is persisted to the BaseApp's
Expand Down Expand Up @@ -361,8 +372,8 @@
}

return &abci.CheckTxResponse{
GasWanted: int64(gasInfo.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(gasInfo.GasUsed), // TODO: Should type accept unsigned ints?
GasWanted: safeInt64FromUint64(gasInfo.GasWanted),
GasUsed: safeInt64FromUint64(gasInfo.GasUsed),
Log: result.Log,
Comment on lines +375 to 377
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Correctly uses saturating conversion when constructing CheckTxResponse.

Covers the success path; the error path already routes through errors.ResponseCheckTxWithEvents which now also saturates. Nice.

Run to spot any remaining direct casts of gas into int64:

🏁 Script executed:

#!/bin/bash
# Find risky int64 casts into GasWanted/GasUsed initializers.
rg -nP -C2 --type=go '(GasWanted|GasUsed)\s*:\s*int64\('

# Find any ABCI response construction bypassing the safe helpers.
rg -nP --type=go -C2 'abci\.(CheckTxResponse|ExecTxResult)\s*{'

Length of output: 4134

Refactor ExecTxResult to use saturating conversions for Gas fields

The success path in deliverTx still uses direct int64 casts, which can overflow. Replace these with the existing safeInt64FromUint64 helper to ensure saturation:

• File baseapp/baseapp.go, around line 723

-   GasWanted: int64(gInfo.GasWanted),
-   GasUsed:   int64(gInfo.GasUsed),
+   GasWanted: safeInt64FromUint64(gInfo.GasWanted),
+   GasUsed:   safeInt64FromUint64(gInfo.GasUsed),

This aligns the success path with the CheckTx responses and error paths that already saturate.

🤖 Prompt for AI Agents 
In baseapp/baseapp.go around line 723, the ExecTxResult success path currently
casts gas fields directly to int64 which can overflow; replace those direct
int64 casts for GasWanted and GasUsed with the existing safeInt64FromUint64
helper (i.e., set GasWanted: safeInt64FromUint64(gasInfo.GasWanted) and GasUsed:
safeInt64FromUint64(gasInfo.GasUsed)) so the values saturate like the CheckTx
and error paths.

Data: result.Data,
Events: sdk.MarkEventsToIndex(result.Events, app.indexEvents),
Expand Down
27 changes: 27 additions & 0 deletions baseapp/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/hex"
"errors"
"fmt"
"math"
"math/rand"
"strconv"
"strings"
Expand Down Expand Up @@ -2592,3 +2593,29 @@ func TestABCI_Race_Commit_Query(t *testing.T) {

require.Equal(t, int64(1001), app.GetContextForCheckTx(nil).BlockHeight())
}

func TestABCI_CheckTx_WithGasOverflow(t *testing.T) {
// Test that CheckTx doesn't panic with large gas values
// This indirectly tests the safe conversion logic

// Test that we can create a response without panic
response := &abci.CheckTxResponse{
GasWanted: int64(math.MaxInt64), // Should be capped at MaxInt64
GasUsed: int64(math.MaxInt64), // Should be capped at MaxInt64
}

require.Equal(t, int64(math.MaxInt64), response.GasWanted)
require.Equal(t, int64(math.MaxInt64), response.GasUsed)

// Test with normal values
normalGasWanted := uint64(1000)
normalGasUsed := uint64(500)

normalResponse := &abci.CheckTxResponse{
GasWanted: int64(normalGasWanted),
GasUsed: int64(normalGasUsed),
}

require.Equal(t, int64(1000), normalResponse.GasWanted)
require.Equal(t, int64(500), normalResponse.GasUsed)
}
Comment on lines 2588 to 2613
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Test does not exercise the overflow path; it only sets int64 fields directly.

This never triggers uint64→int64 conversion, so it can’t catch regressions. Replace with tests that call the constructors performing the conversion.

Apply this diff:

 func TestABCI_CheckTx_WithGasOverflow(t *testing.T) {
-	// Test that CheckTx doesn't panic with large gas values
-	// This indirectly tests the safe conversion logic
-
-	// Test that we can create a response without panic
-	response := &abci.CheckTxResponse{
-		GasWanted: int64(math.MaxInt64), // Should be capped at MaxInt64
-		GasUsed:   int64(math.MaxInt64), // Should be capped at MaxInt64
-	}
-
-	require.Equal(t, int64(math.MaxInt64), response.GasWanted)
-	require.Equal(t, int64(math.MaxInt64), response.GasUsed)
-
-	// Test with normal values
-	normalGasWanted := uint64(1000)
-	normalGasUsed := uint64(500)
-
-	normalResponse := &abci.CheckTxResponse{
-		GasWanted: int64(normalGasWanted),
-		GasUsed:   int64(normalGasUsed),
-	}
-
-	require.Equal(t, int64(1000), normalResponse.GasWanted)
-	require.Equal(t, int64(500), normalResponse.GasUsed)
+	// overflow: uint64 -> int64 should saturate at MaxInt64
+	res := sdkerrors.ResponseCheckTxWithEvents(nil, math.MaxUint64, math.MaxUint64, nil, false)
+	require.Equal(t, int64(math.MaxInt64), res.GasWanted)
+	require.Equal(t, int64(math.MaxInt64), res.GasUsed)
+
+	// normal path
+	res = sdkerrors.ResponseCheckTxWithEvents(nil, 1000, 500, nil, false)
+	require.Equal(t, int64(1000), res.GasWanted)
+	require.Equal(t, int64(500), res.GasUsed)
+
+	// also assert for ExecTxResult path
+	execRes := sdkerrors.ResponseExecTxResultWithEvents(nil, math.MaxUint64, math.MaxUint64, nil, false)
+	require.Equal(t, int64(math.MaxInt64), execRes.GasWanted)
+	require.Equal(t, int64(math.MaxInt64), execRes.GasUsed)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestABCI_CheckTx_WithGasOverflow(t *testing.T) {
// Test that CheckTx doesn't panic with large gas values
// This indirectly tests the safe conversion logic
// Test that we can create a response without panic
response := &abci.CheckTxResponse{
GasWanted: int64(math.MaxInt64), // Should be capped at MaxInt64
GasUsed: int64(math.MaxInt64), // Should be capped at MaxInt64
}
require.Equal(t, int64(math.MaxInt64), response.GasWanted)
require.Equal(t, int64(math.MaxInt64), response.GasUsed)
// Test with normal values
normalGasWanted := uint64(1000)
normalGasUsed := uint64(500)
normalResponse := &abci.CheckTxResponse{
GasWanted: int64(normalGasWanted),
GasUsed: int64(normalGasUsed),
}
require.Equal(t, int64(1000), normalResponse.GasWanted)
require.Equal(t, int64(500), normalResponse.GasUsed)
}
func TestABCI_CheckTx_WithGasOverflow(t *testing.T) {
// overflow: uint64 -> int64 should saturate at MaxInt64
res := sdkerrors.ResponseCheckTxWithEvents(nil, math.MaxUint64, math.MaxUint64, nil, false)
require.Equal(t, int64(math.MaxInt64), res.GasWanted)
require.Equal(t, int64(math.MaxInt64), res.GasUsed)
// normal path
res = sdkerrors.ResponseCheckTxWithEvents(nil, 1000, 500, nil, false)
require.Equal(t, int64(1000), res.GasWanted)
require.Equal(t, int64(500), res.GasUsed)
// also assert for ExecTxResult path
execRes := sdkerrors.ResponseExecTxResultWithEvents(nil, math.MaxUint64, math.MaxUint64, nil, false)
require.Equal(t, int64(math.MaxInt64), execRes.GasWanted)
require.Equal(t, int64(math.MaxInt64), execRes.GasUsed)
}
🤖 Prompt for AI Agents 
In baseapp/abci_test.go around lines 2596 to 2621, the test sets
CheckTxResponse.GasWanted/GasUsed directly as int64 so it never exercises the
uint64→int64 conversion path; update the test to use the functions/constructors
that perform the safe conversion (the same helpers the production code uses) by
passing large uint64 values (including values > MaxInt64) and normal uint64
values into those constructors and asserting the returned CheckTxResponse has
GasWanted/GasUsed capped to MaxInt64 for overflow cases and correctly converted
for normal cases; ensure you assert via the constructor results rather than by
directly assigning int64 fields.

19 changes: 15 additions & 4 deletions types/errors/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
package errors

import (
"math"

abci "github.com/cometbft/cometbft/v2/abci/types"

errorsmod "cosmossdk.io/errors"
)

// safeInt64FromUint64 converts uint64 to int64 with overflow checking.
// If the value is too large to fit in int64, it returns math.MaxInt64.
func safeInt64FromUint64(val uint64) int64 {
Copy link
Contributor

@aljo242 aljo242 Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we should just have this as a common function so we don't duplicate across the code

if val > math.MaxInt64 {
return math.MaxInt64
}
return int64(val)
}

// ResponseCheckTxWithEvents returns an ABCI ResponseCheckTx object with fields filled in
// from the given error, gas values and events.
func ResponseCheckTxWithEvents(err error, gw, gu uint64, events []abci.Event, debug bool) *abci.CheckTxResponse {
Expand All @@ -14,8 +25,8 @@ func ResponseCheckTxWithEvents(err error, gw, gu uint64, events []abci.Event, de
Codespace: space,
Code: code,
Log: log,
GasWanted: int64(gw),
GasUsed: int64(gu),
GasWanted: safeInt64FromUint64(gw),
GasUsed: safeInt64FromUint64(gu),
Events: events,
}
}
Expand All @@ -28,8 +39,8 @@ func ResponseExecTxResultWithEvents(err error, gw, gu uint64, events []abci.Even
Codespace: space,
Code: code,
Log: log,
GasWanted: int64(gw),
GasUsed: int64(gu),
GasWanted: safeInt64FromUint64(gw),
GasUsed: safeInt64FromUint64(gu),
Events: events,
}
}
Expand Down
Loading