Opened 7 years ago

Last modified 7 years ago

#6381 assigned defect

RPC executeQuery is open to SQL Injection

Reported by: carstenklein@… Owned by: Yuji OKAZAKI
Priority: normal Component: TracDependencyPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.11


Please adjust the implementation of the executeQuery Method so that it prevents SQL Injection.

Both parameters "query" and "sort" are not tested against common types of SQL Injection attacks.

E.g. providing for sort a value of e.g. "start_time asc; DELETE FROM ticket WHERE 1=1; DELETE FROM wiki WHERE 1=1;" would remove all of your important ticket and wiki data. Actual table names and field names may be different from the ones used in the above example, but you should get the picture.

Attachments (0)

Change History (5)

comment:1 Changed 7 years ago by Yuji OKAZAKI

(In [7305]) refs #6381

comment:2 Changed 7 years ago by carstenklein@…

Deactivating the RPC functionality is an option, but how about trying to test against for example the possible injection of sql statements?


if ( sort != ):

assertNotAnSqlInjection( query )


sort = DEFAULT

if ( query != ):

assertNotAnSqlInjection( sort )


query = DEFAULT


and (PSEUDO Code here)

def assertNotAnSqlInjection( sql ):

if SELECT, DELETE or UPDATE or CREATE or DROP in ucase(sql) or ucase(sql):

raise SqlInjectionDetectedException(...)

Of course, the assertion should be somewhat more expressive in that other possible injections can also be detected.

comment:3 Changed 7 years ago by Yuji OKAZAKI

Status: newassigned


comment:4 Changed 7 years ago by Yuji OKAZAKI

Thank you, I will try it.

comment:5 Changed 7 years ago by Odd Simon Simonsen

The pattern that should be used is not string substitution, but parameter substition at db level. Use cursor.execute(statement, args) and not cursor.execute(statement % args). The db backends will then handle all the necessary escaping.

Your args array need to contain a matching number of arguments according to the number of %s you put into your string, and there is no need to consider type or special marking of your substitutions in your string other than plain %s (ie. don't use the string substitution ideom of ...version='%s' -just use ...version=%s.


cursor.execute("select * from wiki where name=%s and version=%s", ["WikiStart", 1])

Modify Ticket

as assigned The owner will remain Yuji OKAZAKI.

Add Comment

E-mail address and name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.