s9y / Serendipity

A PHP blog software
https://s9y.org
BSD 3-Clause "New" or "Revised" License
203 stars 86 forks source link

Unhandled mysqli exception in search with invalid search strings #844

Open hannob opened 2 months ago

hannob commented 2 months ago

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 commented 2 months ago

Probably something to catch around https://github.com/s9y/Serendipity/blob/71f2c2dfaed766ed4a33096aed3524794859c0d3/include/genpage.inc.php#L61

onli commented 3 weeks ago

This should be fixed now in current master, thanks to https://github.com/s9y/Serendipity/pull/846 by @GuillaumeValadas

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

hannob commented 3 weeks ago

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/

garvinhicking commented 3 weeks ago

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