Modify

Opened 5 years ago

Last modified 4 years ago

#6381 assigned defect

RPC executeQuery is open to SQL Injection

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

Description

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 5 years ago by okazaki

(In [7305]) refs #6381

comment:2 Changed 5 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?

e.g.

if ( sort != ):

assertNotAnSqlInjection( query )

else:

sort = DEFAULT

if ( query != ):

assertNotAnSqlInjection( sort )

else:

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 4 years ago by okazaki

  • Status changed from new to assigned

ありがとうございます、私はそれを試みるつもりです。

comment:4 Changed 4 years ago by okazaki

Thank you, I will try it.

comment:5 Changed 4 years ago by osimons

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.

Example:

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

Add Comment

Modify Ticket

Action
as assigned .
Author


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

 
Note: See TracTickets for help on using tickets.