-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add enchant_dict_remove_from_session() #17393
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
Conversation
$dicts = enchant_broker_list_dicts($broker); | ||
$newWord = 'myImaginaryWord'; | ||
|
||
$requestDict = enchant_broker_request_dict($broker, $dicts[0]['lang_tag']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind adding a test which includes a null byte in the string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're gonna love this.
Turns out that enchant crashes when you have a null byte in a string, for all the existing methods too...
So the fact that nulls aren't prevented is a bug since the introduction of the enchant extension.
Try it yourself!
<?php
$broker = enchant_broker_init();
$dicts = enchant_broker_list_dicts($broker);
$requestDict = enchant_broker_request_dict($broker, $dicts[0]['lang_tag']);
var_dump(enchant_dict_check($requestDict, "foo\0bar"));
libenchant-CRITICAL **: 19:54:21.646: string_substring: assertion '(offset + len) <= string_length' failed
And then a segfault
I guess I'll make a PR for lower branches to change the "s" to "p".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely, I wonder how many bugs could be found by fuzzing extension functions with strings that contain null bytes.
@YuanchengJiang you might want to try this in your fuzzer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased this and added a commit to properly handle the null bytes.
Will squash ofc on merge.
No description provided.