id	summary	reporter	owner	description	type	status	priority	component	severity	resolution	keywords	cc	release
10023	SQL Injection in acct_mgr.api.AccountManager.lastseen()	hasienda	hasienda	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:\r\n\r\nThe 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.\r\n\r\nExample: http://localhost/admin/accounts/users?user=foobar%27\r\n\r\nThis vulnerability is hard to exploit, because\r\n a. ) one doesn't get feedback about the query result\r\n b. ) one needs access to the useradmin section as a prerequisite\r\n c. ) one is unable to execute multiple statements at a time. (something like `';INSERT INTO permissions values ('bluec0re', 'TRAC_ADMIN')--`` is impossible)\r\n\r\nNevertheless at that place parameter binding should be used as well:\r\n\r\n{{{\r\n#!diff\r\nIndex: acct_mgr/api.py\r\n===================================================================\r\n--- acct_mgr/api.py    (Revision 11513)\r\n+++ acct_mgr/api.py    (Arbeitskopie)\r\n@@ -277,8 +277,10 @@\r\n              WHERE authenticated=1\r\n             """\r\n         if user:\r\n-            sql = "%s AND sid='%s'" % (sql, user)\r\n-        cursor.execute(sql)\r\n+            sql += " AND sid=?"\r\n+            cursor.execute(sql, (user,))\r\n+        else:\r\n+            cursor.execute(sql)\r\n         # Don't pass over the cursor (outside of scope), only it's content.\r\n         res = []\r\n         for row in cursor:\r\n}}}\r\n----\r\nI 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.	defect	closed	high	AccountManagerPlugin	minor	fixed	sql injection security	rjollos, otaku42	0.11