fix: add cycle detection to EIP-712 typed data encoding to prevent stack overflow DoS#210
Conversation
…ack overflow DoS Cyclic type definitions (e.g. A→B→A) caused infinite recursion in EncodeType/encodeValue, leading to an unrecoverable stack overflow. This was reachable via unauthenticated endpoints with a ~200-byte payload. Add ValidateTypeGraph() with DFS cycle detection, called in both UnmarshalJSON and Encode to reject cycles before any recursion begins. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9608992df0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for typeName := range t { | ||
| if err := t.walkTypeGraph(typeName, make(map[string]bool)); err != nil { |
There was a problem hiding this comment.
Because this loops over every entry in Types, Encode and JSON unmarshalling now reject cycles that are never reachable from PrimaryType or EIP712Domain. For example, a signable payload with primary type Mail plus an unused A -> B -> A helper definition now fails before encoding, even though the existing encoder only traverses the domain and primary-type dependency graph and the unused types cannot cause recursion. Limit the DFS roots to the types actually encoded so irrelevant definitions do not break otherwise valid typed data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Unreachable types are irrelevant, I don't think we need to make an exception for them. Open to changing this though, if needed.
2 participants
Summary
A→B→A) cause infinite recursion inEncodeType/encodeValue, leading to an unrecoverable stack overflow (recover()cannot catch it — the process dies)ValidateTypeGraph()using DFS with grey-node cycle detection, called at two entry points:UnmarshalJSON— rejects malicious payloads before any recursive traversalEncode— catches programmatically constructed cyclic typesB[]) are handled by stripping the suffix before graph traversalTest plan