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

db: Guard access to channel_htlcs.malformed_onion with a NULL check #4378

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Feb 8, 2021

We fix it twice: once when accessing the field, and a second time by
inserting the default value 0 on insert. The latter is likely to be
preferred, but the latter former is also need to fix existing wallets.

Fixes: #4363

Reported-by: Zoltán Gálli <@gallizoltan>
Changelog-Fixed: db: Fixed an access to a NULL-field in the channel_htlcs table and resulting warning.

@rustyrussell
Copy link
Contributor

This is a classic case for COMPAT, or even a db migration?

This ensures that after the migration in the previous commit we never
insert a new htlc with a null value.

Fixes: ElementsProject#4363

Reported-by: Zoltán Gálli <@gallizoltan>
Changelog-Fixed: db: Fixed an access to a NULL-field in the `channel_htlcs` table and resulting warning.
@cdecker
Copy link
Member Author

cdecker commented Feb 14, 2021

Good point, the migration allows us not to use the COMPAT flag and frees us from having to check it as well. In combination with the setting of the default value we now always have the value of htlc->badonion set.

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 1033989

@rustyrussell rustyrussell merged commit c8f7cfe into ElementsProject:master Feb 25, 2021
vibhaa pushed a commit to spider-pcn/lightning that referenced this pull request Mar 24, 2021
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.

broken database
2 participants