Skip to content

fix: enforce PubKey-Address binding in LCA evidence validation (backport #2848)#2933

Merged
rootulp merged 2 commits intov0.39.x-celestiafrom
mergify/bp/v0.39.x-celestia/pr-2848
Apr 17, 2026
Merged

fix: enforce PubKey-Address binding in LCA evidence validation (backport #2848)#2933
rootulp merged 2 commits intov0.39.x-celestiafrom
mergify/bp/v0.39.x-celestia/pr-2848

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Apr 16, 2026

Fixes https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-220

Summary

  • validateABCIEvidence checked Address and VotingPower of byzantine validators but did not verify that PubKey.Address() matched the Address field
  • Since ABCI() derives misbehavior addresses from PubKey.Address() (via TM2PB.Validator), an attacker could submit evidence with swapped PubKeys that passes verification but redirects slash/jail/tombstone attribution to innocent validators
  • Add a check that ev.ByzantineValidators[i].PubKey.Address() equals the expected address from the node's own validator set

Test plan

  • TestValidateABCIEvidenceRejectsMismatchedPubKey: creates valid lunatic evidence, swaps PubKeys in ByzantineValidators (keeping Address/VotingPower intact), asserts VerifyLightClientAttack rejects it. Verified this test fails before the fix and passes after.
  • TestABCIEvidenceAddressDerivedFromPubKey: confirms ABCI() derives addresses from PubKey.Address() not the Address field, demonstrating why the binding check is necessary.
  • Full evidence package test suite passes.

🤖 Generated with Claude Code


Open with Devin
This is an automatic backport of pull request #2848 done by [Mergify](https://mergify.com).
Open with Devin

Fixes
https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-220

## Summary

- `validateABCIEvidence` checked `Address` and `VotingPower` of
byzantine validators but did not verify that `PubKey.Address()` matched
the `Address` field
- Since `ABCI()` derives misbehavior addresses from `PubKey.Address()`
(via `TM2PB.Validator`), an attacker could submit evidence with swapped
PubKeys that passes verification but redirects slash/jail/tombstone
attribution to innocent validators
- Add a check that `ev.ByzantineValidators[i].PubKey.Address()` equals
the expected address from the node's own validator set

## Test plan

- [x] `TestValidateABCIEvidenceRejectsMismatchedPubKey`: creates valid
lunatic evidence, swaps PubKeys in ByzantineValidators (keeping
Address/VotingPower intact), asserts `VerifyLightClientAttack` rejects
it. Verified this test **fails before the fix** and **passes after**.
- [x] `TestABCIEvidenceAddressDerivedFromPubKey`: confirms `ABCI()`
derives addresses from `PubKey.Address()` not the `Address` field,
demonstrating why the binding check is necessary.
- [x] Full `evidence` package test suite passes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- devin-review-badge-begin -->

---

<a
href="https://app.devin.ai/review/celestiaorg/celestia-core/pull/2848"
target="_blank">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img
src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1"
alt="Open with Devin">
  </picture>
</a>
<!-- devin-review-badge-end -->

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
(cherry picked from commit 5fef39f)
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +1 to +3
- `[evidence]` Add PubKey-to-Address binding check in
`validateABCIEvidence` to prevent LightClientAttackEvidence with swapped
PubKeys from redirecting slash attribution to innocent validators.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changelog filename missing required issue/PR number prefix

The changelog file is named lca-pubkey-address-binding.md but both CONTRIBUTING.md and CLAUDE.md require the format {issue-or-pr-number}-{description}.md. Virtually all other changelog entries in the repository (e.g., 2913-mempool-user-tx-latency-metric.md, 3092-consensus-timeout-ticker-data-race.md) follow this convention. For PR #2933, the file should be named 2933-lca-pubkey-address-binding.md.

Prompt for agents
Rename the changelog file from .changelog/unreleased/bug-fixes/lca-pubkey-address-binding.md to .changelog/unreleased/bug-fixes/2933-lca-pubkey-address-binding.md (or use the relevant issue number instead of the PR number). Both CONTRIBUTING.md and CLAUDE.md require the format {issue-or-pr-number}-{description}.md for changelog entries.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@rootulp rootulp self-assigned this Apr 17, 2026
@rootulp rootulp merged commit 27ad9a8 into v0.39.x-celestia Apr 17, 2026
22 checks passed
@rootulp rootulp deleted the mergify/bp/v0.39.x-celestia/pr-2848 branch April 17, 2026 15:57
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.

2 participants