Skip to content

Conversation

@odaysec
Copy link

@odaysec odaysec commented Jan 20, 2026

Fix error handling for hex value decoding in Long type. In general, when parsing an integer into a wider, unsigned type (here uint64) and then converting it to a narrower or signed type (int64), you must ensure the parsed value is within the target type’s range before conversion. For signed 64-bit (int64), that means checking that the uint64 value is not greater than math.MaxInt64. If the value is out of range, you should return an error instead of performing the unsafe cast.

For this specific case, the best minimal fix is to add an explicit range check in (*Long).UnmarshalGraphQL before converting the uint64 returned by hexutil.DecodeUint64 into Long (which is int64). If value > math.MaxInt64, UnmarshalGraphQL should return an error indicating that the hex long is out of range for Long. This preserves existing behavior for all in-range values and only changes behavior when the user provides an out-of-range hex value, which currently would silently wrap.

Concretely:

  • In graphql/graphql.go, inside UnmarshalGraphQL, in the case string: / if strings.HasPrefix(input, "0x") branch, after value, err := hexutil.DecodeUint64(input) and before *b = Long(value), add:
    • a check if value > math.MaxInt64 { return fmt.Errorf("hex value %s overflows Long", input) }.
  • To use math.MaxInt64, add an import of the standard library package math to graphql/graphql.go.
  • No changes are needed in common/hexutil/hexutil.go, since it already correctly parses into uint64; the unsafe step is in graphql/graphql.go.

Changes

If a string is parsed into an int using strconv.Atoi, and subsequently that int is converted into another integer type of a smaller size, the result can produce unexpected values. This also applies to the results of strconv.ParseInt and strconv.ParseUint when the specified size is larger than the size of the type that number is converted to.

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Changes only for a subset of nodes

Checklist

  • I have added at least 2 reviewer or the whole pos-v1 team
  • I have added sufficient documentation in code
  • I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply
  • Created a task in Jira and informed the team for implementation in Erigon client (if applicable)
  • Includes RPC methods changes, and the Notion documentation has been updated

Cross repository changes

  • This PR requires changes to heimdall
    • In case link the PR here:
  • This PR requires changes to matic-cli
    • In case link the PR here:

Testing

  • I have added unit tests
  • I have added tests to CI
  • I have tested this code manually on local environment
  • I have tested this code manually on remote devnet using express-cli
  • I have tested this code manually on amoy
  • I have created new e2e tests into express-cli

References

Wikipedia Integer overflow
Go language specification Integer overflow
Documentation for strconv.Atoi
Documentation for strconv.ParseInt
Documentation for strconv.ParseUint

odaysec

This comment was marked as resolved.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant