Skip to content
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.

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

header: Do not panic on data root mismatch on unmarshal #2555

Closed
renaynay opened this issue Aug 10, 2023 · 5 comments · Fixed by #2558
Closed

header: Do not panic on data root mismatch on unmarshal #2555

renaynay opened this issue Aug 10, 2023 · 5 comments · Fixed by #2558
Assignees
Labels
area:header Extended header bug Something isn't working

Comments

@renaynay
Copy link
Member

As the fields are public, if a full node or light node is sent an ExtendedHeader with mismatched Row/Col roots that don't compute to the DataRoot, the node will panic.

We should do a severe WARN log and ban the peer instead.

@renaynay renaynay added bug Something isn't working area:header Extended header labels Aug 10, 2023
@renaynay renaynay self-assigned this Aug 10, 2023
@Wondertan
Copy link
Member

Supposed to be fixed by #2220

@renaynay
Copy link
Member Author

@Wondertan I'm working on a fix right now that implements a custom error - bridge nodes should panic as they receive blocks from a "trusted" source.

Change is almost the same as yours, would you be ok rebasing on it?

@Wondertan
Copy link
Member

I don't mind rebasing, if you want to move the panic from Validate to CoreListener

@renaynay
Copy link
Member Author

Yes pls :)

@renaynay
Copy link
Member Author

renaynay added a commit that referenced this issue Aug 11, 2023
#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves #2555
renaynay added a commit to renaynay/celestia-node that referenced this issue Aug 23, 2023
celestiaorg#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves celestiaorg#2555
walldiss pushed a commit to walldiss/celestia-node that referenced this issue Sep 22, 2023
celestiaorg#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves celestiaorg#2555

(cherry picked from commit 87e9500)
walldiss pushed a commit that referenced this issue Sep 22, 2023
#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves #2555

(cherry picked from commit 87e9500)
walldiss pushed a commit to walldiss/celestia-node that referenced this issue Sep 22, 2023
celestiaorg#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves celestiaorg#2555

(cherry picked from commit 87e9500)
walldiss pushed a commit to walldiss/celestia-node that referenced this issue Sep 25, 2023
celestiaorg#2558)

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves celestiaorg#2555

(cherry picked from commit 87e9500)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:header Extended header bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants