pluginsGLPI / formcreator

GLPI Plugin Formcreator (DOWNLOAD : https://github.com/pluginsGLPI/formcreator/releases)
http://www.teclib-edition.com
GNU General Public License v3.0
174 stars 125 forks source link

SQL Error when updating a form issue that has a single quote character in name #3409

Closed keguira closed 1 year ago

keguira commented 1 year ago

Describe the bug

No user complained but i may have miss something and i'm doing a review of the logs.

I can see a lots of error like this one :

SQL: UPDATE `glpi_plugin_formcreator_issues` SET `status` = '101', `name` = 'Demande d'accès - APP', `comment` = '<h2>Modification d'accès</h2>
.....
 Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'accès - APP', `comment` = '<h2>Modification d'' at line 1

the single quote generate an error and i can see that the single quote is not html-escaped or backslahed.

To Reproduce Steps to reproduce the behavior:

  1. Create a form with a target that has a "ticket title" and a description containing a single quote in it.
  2. Use it

Expected behavior No SQL error should be generated

Screenshots

GLPI / Plugins (please complete the following information):

btry commented 1 year ago

Hi

I think this error occurs when a ticket is updated. You will find a backtrace with the error. Please show it. I need it to locate where the faulty code. I think it will be easy to fix.

btry commented 1 year ago

The PR currently uses a better method for quote escaping; but I'm not sure I worked in the right place. I'm waiting for your backtrace.

keguira commented 1 year ago

Hi

I think this error occurs when a ticket is updated. You will find a backtrace with the error. Please show it. I need it to locate where the faulty code. I think it will be easy to fix.

sorry, it was a bad copy past, here is the full backtrace :

  Backtrace :
  src/DBmysql.php:1468
  src/CommonDBTM.php:686                             DBmysql->update()
  src/CommonDBTM.php:1675                            CommonDBTM->updateInDB()
  marketplace/formcreator/hook.php:607               CommonDBTM->update()
  src/Plugin.php:1665                                plugin_formcreator_hook_update_ticketvalidation()
  src/CommonDBTM.php:1680                            Plugin::doHook()
  front/commonitilvalidation.form.php:113            CommonDBTM->update()
  front/ticketvalidation.form.php:40                 include()
  public/index.php:82                                require()
  {"user":"1237@glpi-server"}
btry commented 1 year ago

I updated the above PR. You may try it and tell if the issue is solved.

It appears that the error occurs when the someone updates the validation of a ticket.

keguira commented 1 year ago

Ok thank you, i'll try to test it this week-end or next week

keguira commented 1 year ago

Ok, tested this morning. I do not have the SQL error anymore But I cannot see anything in the app that should the diff. As i'm not fully aware of the domain impact, what is the purpose of this query and what does it handle / change ? I would also validate that's it's doing it correctly and communicate adequately to the end users.

btry commented 1 year ago

The query updates the status of the issue associated to a ticket when this ticket gets a validation status update.

btry commented 1 year ago

More details : the query is created by updating more columns than needed. Unfortunately, some values were not properly escaped. The fix changes the query to update only the status, which is enough in the context of a ticket validation update.

keguira commented 1 year ago

ok, i do not see anything wrong. I've created multiple tickets and my status are ok

btry commented 1 year ago

Great, I merge the fix, it will available in the next release. Thank you for your feedback.