Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect against malicious SQL that causes damage even though our DB is immutable #39

Closed
simonw opened this issue Oct 25, 2017 · 4 comments

Comments

@simonw
Copy link
Owner

simonw commented Oct 25, 2017

I’m currently operating under the assumption that it’s safe to allow arbitrary SQL statements because we are dealing with an immutable database. But this might not be the case - there are some pretty weird SQLite language extensions (ATTACH, PRAGMA etc) and I’m not certain they cannot be used to break things in a way that would affect future requests to the API.

Solution: provide a “safe mode” option which disables the ?sql= mechanism. This still leaves the URL filter lookups, so I need to make sure that those are “safe”.

In the future I may also implement a whitelist option where datasets can be configured to only allow specific filters against specific columns.

@simonw simonw added this to the Ship v1 milestone Oct 25, 2017
@simonw simonw added the medium label Oct 25, 2017
@simonw
Copy link
Owner Author

simonw commented Oct 25, 2017

It certainly looks like some of the stuff in https://sqlite.org/pragma.html could be used to screw around with things. Example: PRAGMA case_sensitive_like = 1 - would that affect future queries?

@simonw
Copy link
Owner Author

simonw commented Oct 25, 2017

Could I use https://sqlparse.readthedocs.io/en/latest/ to parse incoming statements and ensure they are pure SELECTs? Would that prevent people from using a compound SELECT statement to trigger an evil PRAGMA of some sort?

@simonw simonw changed the title “Safe” mode Protect against malicious SQL that causes damage even though our DB is immutable Oct 25, 2017
@simonw
Copy link
Owner Author

simonw commented Oct 26, 2017

It looks like I should double quote my columns and ensure they are correctly escaped https://blog.christosoft.de/2012/10/sqlite-escaping-table-acolumn-names/ - hopefully using ? placeholders for column names will work. I should use ? for tables too.

@simonw
Copy link
Owner Author

simonw commented Oct 31, 2017

Here’s how I can (I think) provide safe execution of arbitrary SQL while blocking PRAGMA calls: let people use names parameters in their SQL and apply strict filtering to the SQL query but not to the parameter values.

cur.execute(
    "select * from people where name_last=:who and age=:age", {
        "who": who,
        "age": age
})

In URL form:

?sql=select...&who=Terry&age=34

Now we can apply strict, dumb validation rules to the SQL part while allowing anything in the named queries - so people can execute a search for PRAGMA without being able to execute a PRAGMA statement.

@simonw simonw closed this as completed in 186c513 Nov 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant