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

Unhandled mysqli exception in search with invalid search strings #844

Open
hannob opened this issue Jul 10, 2024 · 4 comments
Open

Unhandled mysqli exception in search with invalid search strings #844

hannob opened this issue Jul 10, 2024 · 4 comments

Comments

@hannob
Copy link
Contributor

hannob commented Jul 10, 2024

I'm monitoring my PHP error logs, which often helps me identify bugs in PHP applications.

Since a while, I'm regularly seeing unhandled mysqli exceptions by serendipity, like this:

PHP Fatal error:  Uncaught mysqli_sql_exception: syntax error, unexpected '-' in [path]/include/db/mysqli.inc.php:68
Stack trace:
#0 [path]/include/db/mysqli.inc.php(68): mysqli_query()
#1 [path]/include/functions_entries.inc.php(953): serendipity_db_query()
#2 [path]/include/genpage.inc.php(61): serendipity_searchEntries()
#3 [path]/include/functions_routing.inc.php(199): include('...')
#4 [path]/index.php(93): serveSearch()
#5 {main}
  thrown in [path]/include/db/mysqli.inc.php on line 68

What happens is that the search function of s9y can pass invalid search strings to mysql, and that causes this exception.
You can also test this on the main s9y blog, by passing a search string like "test -" into the search form:
https://blog.s9y.org/index.php?serendipity%5Baction%5D=search&serendipity%5BsearchTerm%5D=test+-&serendipity%5BsearchButton%5D=Los%21

This causes an error 500 for the user.

I think this should be caught somewhere and the user should get some form of error. I've looked a bit where to do that, but haven't yet written a patch, as it's not entirely obvious where to best handle this. The code in serendipity_searchEntries() is generic for various DB backends, but serendipity_db_query() doesn't really know that the query is a search, which could cause such errors.

Reporting it here, so maybe others can have a look.

@onli
Copy link
Member

onli commented Jul 10, 2024

Probably something to catch around

$r = serendipity_searchEntries($serendipity['GET']['searchTerm']);

GuillaumeValadas added a commit to GuillaumeValadas/Serendipity that referenced this issue Jul 16, 2024
GuillaumeValadas added a commit to GuillaumeValadas/Serendipity that referenced this issue Jul 16, 2024
GuillaumeValadas added a commit to GuillaumeValadas/Serendipity that referenced this issue Jul 17, 2024
…arch anyway + change alias to mysqli_real_escape_string
GuillaumeValadas added a commit to GuillaumeValadas/Serendipity that referenced this issue Jul 17, 2024
…not + change regex to only catch BOOLEAN Operator that prefix a word
GuillaumeValadas added a commit to GuillaumeValadas/Serendipity that referenced this issue Jul 23, 2024
onli pushed a commit that referenced this issue Aug 15, 2024
#846)

* Fix search issue with special characters, and escape them in SQL. #844

* Update functions_entries.inc.php

Co-authored-by: Garvin Hicking <38074677+fe-hicking@users.noreply.github.com>

* Update functions_entries.inc.php

Co-authored-by: Garvin Hicking <38074677+fe-hicking@users.noreply.github.com>

* #844 Simplify code removing If statement because we escape term search anyway + change alias to mysqli_real_escape_string

* Issue #844 Bring back if statement to switch over boolean mode or not + change regex to only catch BOOLEAN Operator that prefix a word

* Update regex boolean mode is trigger only on operator followed by words

* Fix typo on regex

* #844 add another regex to avoid boolean operator alone that could lead to error

---------

Co-authored-by: Garvin Hicking <blog@garv.in>
Co-authored-by: Garvin Hicking <38074677+fe-hicking@users.noreply.github.com>
@onli
Copy link
Member

onli commented Aug 15, 2024

This should be fixed now in current master, thanks to #846 by @GuillaumeValadas

Thanks for the report @hannob ! I'll close here already, we can re-open if the issue remains or returns.

@onli onli closed this as completed Aug 15, 2024
@hannob
Copy link
Contributor Author

hannob commented Aug 17, 2024

Interestingly, I am still seeing such an error (longer stack trace this time) when applying this patch, but only on one of my s9y installations...
I'm not sure what can cause the different behavior... Any pointers appreciated.
The installation were I still see it is older (i.e. upgraded from older s9y versions, possibly something different in the mysql db?), and has a custom theme.

Error new:

PHP Fatal error:  Uncaught mysqli_sql_exception: syntax error, unexpected $end in [path]/include/db/mysqli.inc.php:68
Stack trace:
#0 [path]/include/db/mysqli.inc.php(68): mysqli_query()
#1 [path]/plugins/serendipity_event_staticpage/serendipity_event_staticpage.php(2874): serendipity_db_query()
#2 [path]/plugins/serendipity_event_staticpage/serendipity_event_staticpage.php(3228): serendipity_event_staticpage->showSearch()
#3 [path]/include/plugin_api.inc.php(1188): serendipity_event_staticpage->event_hook()
#4 [path]/include/functions_smarty.inc.php(587): serendipity_plugin_api::hook_event()
#5 [path]/templates_c/hanno/b8/10/75/b81075def93dbf7b79cf5e8fe6a914a2cba9e9b3_0.file.entries.tpl.php(351): serendipity_smarty_hookPlugin()
#6 [path]/bundled-libs/smarty/smarty/libs/sysplugins/smarty_template_resource_base.php(123): content_66c03d949372f7_73719513()
#7 [path]/bundled-libs/smarty/smarty/libs/sysplugins/smarty_template_compiled.php(114): Smarty_Template_Resource_Base->getRenderedTemplateCode()
#8 [path]/bundled-libs/smarty/smarty/libs/sysplugins/smarty_internal_template.php(217): Smarty_Template_Compiled->render()
#9 [path]/bundled-libs/smarty/smarty/libs/sysplugins/smarty_internal_templatebase.php(238): Smarty_Internal_Template->render()
#10 [path]/bundled-libs/smarty/smarty/libs/sysplugins/smarty_internal_templatebase.php(116): Smarty_Internal_TemplateBase->_execute()
#11 [path]/include/functions_smarty.inc.php(82): Smarty_Internal_TemplateBase->fetch()
#12 [path]/include/functions_entries.inc.php(1364): serendipity_smarty_fetch()
#13 [path]/include/genpage.inc.php(98): serendipity_printEntries()
#14 [path]/include/functions_routing.inc.php(18): include('...')
#15 [path]/index.php(104): serveIndex()
#16 {main}
  thrown in [path]/include/db/mysqli.inc.php on line 68

s9y installation where error still shows up at https://blog.hboeck.de/ - installation where fix appears to work at https://betterscience.org/

@hannob hannob reopened this Aug 17, 2024
@garvinhicking
Copy link
Member

Looks as if it's related to the staticpage plugin? That one may have that search logic wrong, too...

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

No branches or pull requests

3 participants