In the dawn of 2012-04-25 this claim was brought privately to my attention by Timo "bluec0re" Schmid. The following is a rough translation of the German original email message:
The AccountManagerPlugin for Trac includes an SQL injection vulnerability in the user admin page, more specifically in ap.py:last_seen. There the username is directly included into the SQL statement.
Example: http://localhost/admin/accounts/users?user=foobar%27
This vulnerability is hard to exploit, because
- ) one doesn't get feedback about the query result
- ) one needs access to the useradmin section as a prerequisite
- ) one is unable to execute multiple statements at a time. (something like ';INSERT INTO permissions values ('bluec0re', 'TRAC_ADMIN')--` is impossible)
Nevertheless at that place parameter binding should be used as well:
Index: acct_mgr/api.py
===================================================================
--- acct_mgr/api.py (Revision 11513)
+++ acct_mgr/api.py (Arbeitskopie)
@@ -277,8 +277,10 @@
WHERE authenticated=1
"""
if user:
- sql = "%s AND sid='%s'" % (sql, user)
- cursor.execute(sql)
+ sql += " AND sid=?"
+ cursor.execute(sql, (user,))
+ else:
+ cursor.execute(sql)
# Don't pass over the cursor (outside of scope), only it's content.
res = []
for row in cursor:
I replicate this information for reference, because I adhere to a strict don't-hide-security-problems policy. IMHO this is the only responsible way to go for a component like AccountManager.
