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

Reject outbound channels if the total reserve is larger than funding #1463

Conversation

TheBlueMatt
Copy link
Collaborator

In 2826af7 we fixed a fuzz crash
in which the total reserve values in a channel were greater than
the funding amount, checked when an incoming channel is accepted.

This, however, did not fix the same issue for outbound channels,
where a peer can accept a channel with a nonsense reserve value in
the accept_channel message. The full_stack_target fuzzer
eventually found its way into the same issue, which this resolves.

Thanks (again) to Chaincode Labs for providing the fuzzing
resources which found this bug!

In 2826af7 we fixed a fuzz crash
in which the total reserve values in a channel were greater than
the funding amount, checked when an incoming channel is accepted.

This, however, did not fix the same issue for outbound channels,
where a peer can accept a channel with a nonsense reserve value in
the `accept_channel` message. The `full_stack_target` fuzzer
eventually found its way into the same issue, which this resolves.

Thanks (again) to Chaincode Labs for providing the fuzzing
resources which found this bug!
@TheBlueMatt TheBlueMatt added this to the 0.0.107 milestone May 2, 2022
Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

ACK f2de2f3

Another one for the fuzz trophy cabinet 🏆

@dunxen
Copy link
Contributor

dunxen commented May 3, 2022

Would some docs like in L814 need to be changed to make clear the even lower upper bound for the reserve?

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

LGTM!

@TheBlueMatt
Copy link
Collaborator Author

Would some docs like in L814 need to be changed to make clear the even lower upper bound for the reserve?

I think the issue here is more that the fuzzer is opening channels with the full value of the channel as reserve, or close to it, less that our reserve value is higher than it should be.

@valentinewallace valentinewallace merged commit d96a492 into lightningdevkit:main May 5, 2022
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.

5 participants