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

cmd/otk-*: do basic input validation to avoid crashing too easily #896

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Sep 3, 2024

This commit is a lesson from osbuild/otk#186 as it is right now extremly easy to crash the otk disk externals with bad inputs.

This commit adds basic input validation, it's probably a good idea to also go deeper into the library and make it less trusting about pointers but that is (slightly) orthogonal.

Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

Do we want this right now or do we want to postpone this to do the whole schema dance (which is probably going to happen in the future)?

@mvo5
Copy link
Contributor Author

mvo5 commented Sep 3, 2024

Do we want this right now or do we want to postpone this to do the whole schema dance (which is probably going to happen in the future)?

I don't see a downside in merging it now, we can expand Validate() in otkdisk as needed and the "frontends" are calling it in the right place already in this commit. But maybe I'm missing something?

This commit is a lesson from osbuild/otk#186
as it is right now extremly easy to crash the otk disk externals
with bad inputs.

This commit adds basic input validation, it's probably a good
idea to also go deeper into the library and make it less trusting
about pointers but that is (slightly) orthogonal.
@bcl bcl force-pushed the crash-less-in-otk-externals branch from da422b5 to b617d27 Compare September 10, 2024 01:06
@achilleas-k achilleas-k added this pull request to the merge queue Sep 10, 2024
Merged via the queue into osbuild:main with commit dc9903b Sep 10, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants