SqlMacro security problem / sql injection

[martin.p.wyser@…]

The SqlMacro plugin checks that the query (from the user) starts with \s*SELECT, but I can enter a select statement first, and then tack on other statements with a semicolon in postgres, like this:

select 1;
create table my_table (...);
insert into my_table (...) values (...);

The right solution to this is not to parse the statement, but use a connection with a read-only user, or to accept the insecurity and not to check the statement.

[Jun Omae]

We could use SET TRANSACTION READ ONLY to make the transaction read-only. Try the attached patch.

P.S. The plugin doesn't support Trac 1.6 and Python 3 yet. The patch includes the fix to adapt to Trac 1.6 and Python 3.

[anonymous]

Hi Jun Thanks for the patch. I have already checked the docs, and your suggestion won't work. What I do is:

SELECT 1 WHERE false; -- do nothing
ROLLBACK; -- closes the read-only transaction
START;
INSERT ...; -- the evil stuff, also UPDATE, DELETE, CREATE TABLE, ...
COMMIT; -- make the rollback from the plugin ineffective

So, the transaction can be controlled. The problem is the injection of semicolon The only chance is a read-only user. This is probably also with the use of SQL in custom reports, but I have not yet checked this out. I will, later, and also consider you patch to see where the necessary code would need inserting.

[Jun Omae]

Thanks for the patch. I have already checked the docs, and your suggestion won't work. What I do is:

SELECT 1 WHERE false; -- do nothing
ROLLBACK; -- closes the read-only transaction
START;
INSERT ...; -- the evil stuff, also UPDATE, DELETE, CREATE TABLE, ...
COMMIT; -- make the rollback from the plugin ineffective

Oh.... Such multiple statements should be rejected like this:

-        sql = unicode(args).strip()
+        sql = text_type(args).strip().rstrip(';')
+        if ';' in sql:
+            raise ValueError('You can only execute one statement')

So, the transaction can be controlled. The problem is the injection of semicolon The only chance is a read-only user. This is probably also with the use of SQL in custom reports, but I have not yet checked this out. I will, later, and also consider you patch to see where the necessary code would need inserting.

This plugin has [sqlquery] url option which is for database connection string for executing the given query. You could block the attacks using the option with read-only user.