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

138
Defect
arturo182
Jack
2.0
0.4
Plugins
Fixed
Normal
Normal
8 years ago
5 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

8 years and 11 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.

8 years and 11 months ago by Jack

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

8 years and 10 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.

8 years and 10 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.

8 years and 10 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.

8 years and 10 months ago by Jack

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

8 years and 10 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.

8 years and 10 months ago by arturo182

  • Reopened ticket as Reopened

8 years and 10 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]

8 years and 10 months ago by Jack

What code is your plugin using?

8 years and 10 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.

8 years and 10 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.