fix(packer): use Uint64() to prevent silent overflow on GasAllocated#21412
fix(packer): use Uint64() to prevent silent overflow on GasAllocated#21412Aboudjem wants to merge 2 commits intosmartcontractkit:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a silent integer overflow bug in UnpackCheckResult in the OCR2 Keeper V21 ABI packer. Previously, the GasAllocated field (a uint64) was populated via big.Int.Int64(), which silently truncates values above math.MaxInt64 (2^63-1) and returns a negative value that wraps to a large positive uint64. The fix extracts the *big.Int, validates it via IsUint64(), and returns an error with a proper ineligible check result if it overflows.
Changes:
- Extracts
GasAllocatedas a*big.Intbefore conversion, validates range withIsUint64(), and switches fromInt64()toUint64()for the safe cast.
| if !gasAllocatedBig.IsUint64() { | ||
| return GetIneligibleCheckResultWithoutPerformData(payload, UpkeepFailureReasonNone, PackUnpackDecodeFailed, false), | ||
| fmt.Errorf("upkeepId %s GasAllocated value %s overflows uint64", payload.UpkeepID.String(), gasAllocatedBig.String()) | ||
| } |
There was a problem hiding this comment.
The new overflow check path lacks a unit test. The existing test file (packer_test.go) has dedicated tests for the decode failed and unpack failed error paths in UnpackCheckResult, but there is no test case covering the new GasAllocated overflows uint64 error path. A test case should be added with a raw hex payload whose out[4] encodes a value larger than math.MaxUint64 (e.g., a 256-bit integer with bits set above the 64-bit boundary), verifying that the error is returned and the result has PipelineExecutionState set to PackUnpackDecodeFailed.
There was a problem hiding this comment.
The test is already there - see the "gas allocated overflow" case in packer_test.go (added in commit 875fbcb). It encodes GasAllocated as 2^64 (math.MaxUint64+1) and asserts the overflow error.
Summary
UnpackCheckResultinpacker.gousesbig.Int.Int64()to populate theuint64fieldGasAllocated. If the on-chain value exceedsmath.MaxInt64(2^63-1),Int64()silently truncates and returns a negative value, which then wraps to a large positiveuint64. This could cause upkeep gas budgets to be wildly incorrect.The fix extracts the
*big.Int, checksIsUint64(), and returns an error if the value overflows. Otherwise it uses.Uint64()for a safe conversion.Changes
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/packer.go- 7 lines added, 1 removedTest plan
go buildpasses on the target package