stripslashes in fishhook.class.php and admincp/plugins.php

138
Defect
arturo182
Jack
2.0
0.4
Plugins
Fixed
Normal
Normal
7 years ago
3 years ago
0

Description

In line 27 of fishhook.class.php and line 522 of admincp/plugins.php you use stripslashes on hook's code and I don't think that's the right thing to do cause I was just trying to create a bbcode plugin and couldn't do it with stripslashes in those files, when I removed them my plugin works ok.

Attachments

Ticket History

7 years and 6 months ago by Jack

  • Closed ticket as Invalid

stripslashes() isn't used on the code, mysql_real_escape_string() is, to secure the query. Removing this could create a security hole in the Plugin Hook creator.

7 years and 6 months ago by Jack

I am open to suggestions on how to fix the issues the mysql_real_escape_string() and stripslashes() causes.

7 years and 6 months ago by arturo182

It's true that removing stripslashes creates a security hole but you have to remember that the only one allowed to add hooks is the administrator and I don't think he will want to hack his own website.

7 years and 6 months ago by arturo182

Same goes for exporting plugins, there's stripslashes there and it makes exporting useless cause it strips all slashes and makes the plugin code unusable.

7 years and 6 months ago by arturo182

Now that I think about it stripslashes in hook edition code can be replaced by htmlentities(), it makes the text safe and doesn't break the code when saving. With those changes I finished by bbcode plugin but I can't release it cause it won't work on the original code.

7 years and 6 months ago by Jack

I went ahead and removed the escape string and stripslashes stuff, lets see how this goes...

7 years and 6 months ago by arturo182

You shouldn't have removed escaping strings in queries cause now when there's a single quote in code the hook's code, the query will be invalid, just remove stripslashes() but keep res().

Cause when you escape a string it adds slashes before single quotes but those quotes aren't saved in the database, it's just for the query.

7 years and 6 months ago by arturo182

  • Reopened ticket as Reopened

7 years and 6 months ago by Jack

Leaving the real escape string function will add turn ' and " into \' and \" so without stripslashes() the plugin code can't be executed.

I had no problems when testing this code with the db->res() and stripslashes() functions enabled: [code]if(isset($_REQUEST['test'])) { die("test"); }[/code]

7 years and 6 months ago by Jack

What code is your plugin using?

7 years and 6 months ago by arturo182

I'm sorry but that's not true, about the res function, it adds slashes before quotes but those slashes are not saved in the database, I just tried this on my server: <pre>mysql> insert into traq2_users (username) VALUES ('te\'st'); Query OK, 1 row affected, 3 warnings (0.00 sec)

mysql> select * from traq2_users; +----+-----------+------------------------------------------+-----------+-------------------+----------+------------------------------------------+ | id | username | password | name | email | group_id | sesshash | +----+-----------+------------------------------------------+-----------+-------------------+----------+------------------------------------------+ | 1 | arturo182 | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx | arturo182 | [email protected] | 1 | 8d645dd17dca37dddfaac25e10fa9f4b81dedc53 | | 3 | te'st | | | | 2 | 0 | +----+-----------+------------------------------------------+-----------+-------------------+----------+------------------------------------------+ 2 rows in set (0.00 sec)</pre>

So you can use res and not use stripslashes.

Also the example you wrote doesn't use slashes so stripslashes() doesn't break it, but if you had something like: <pre>mysql_query("SELECT * FROM users WHERE name='te\'st';");</pre> stripslashes would remove slash in the query and make it invalid.

7 years and 6 months ago by Jack

  • Closed ticket as Fixed

That was rather strange, when I tested it before it added slashes to the code, but now it's not.

My apologies, db->res() added back to queries.