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

Fix crash on invalid subtype indication. #1082

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

NikLeberg
Copy link
Contributor

Continuation of the fuzzing crashes found by @avelure.
See #1038 (comment).

In resolve_resolution we expect rname to be of kind T_REF or T_ELEM_RESOLUTION otherwise an assert triggers. I simply added a check for the kind and prevent calling resolve_resolution. If this erroreous input is now given, the parser no longer crashes but instead maneuvers completely off the line and everything afterwards may generate lots of other errors. For such a rare and mean input I think its ok though. What do you think?

Cheers

@nickg
Copy link
Owner

nickg commented Dec 1, 2024

I think it would be better to adjust resolve_resolution instead so that it handles this case. Something like

switch (tree_kind(rname)) {
case T_ELEM_RESOLUTION:
   ...
case T_REF:
   ...
default:
   error_at(tree_loc(rname), "not a valid resolution function name");
   tree_set_type(rname, type_new(T_NONE));
   break;

@NikLeberg
Copy link
Contributor Author

NikLeberg commented Dec 2, 2024

Noted and changed.
Thanks for the pointers. I can now finally understand what the heck that all actually was and why the error was triggered. I redid the test in a cleaner way.

@nickg nickg merged commit 9a40363 into nickg:master Dec 3, 2024
12 checks passed
@NikLeberg NikLeberg deleted the fuzz_a3855bb branch December 3, 2024 22:21
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