Hacker News Reader

Comment by nicolaslem | Hacker News Reader

Isn't using f-strings to build SQL queries a large footgun? I can imagine a junior developer introducing a SQL injection by not completely grasping the sqlbind API.

Using the quickstart example, visually both lines are very similar and both are functional, but one of them may introduce a SQL injection:

    sql = f'SELECT * FROM table WHERE field1 = {q/value1} AND field2 > {q/value2}'
    sql = f'SELECT * FROM table WHERE field1 = {value1} AND field2 > {value2}'

View on HN

Proposed safe(r) API via decorated methods:

    QParams = sqlbind.Dialect.some_dialect

    @QParams
    def make_my_query(value1: str, value2: int):
        return f'SELECT * FROM table WHERE field1 = {value1} AND field2 > {value2}'

    data = connection.execute(*make_my_query(foo, bar))

Obviously this wouldn't work as-is, QParams would need to be modified to support this. The decorator would wrap the method, sanitising the args before they're even passed to the wrapped function.

Edit: actually, I might've misunderstood what sqlbind is doing internally, maybe this approach doesn't quite make sense

Edit 2: Like so: https://github.com/baverman/sqlbind/issues/1

If I understand your code correctly, you are bypassing DBAPI's binding functionality and opening yourself up for SQL injection.

    QParams = sqlbind.Dialect.some_dialect

    @QParams
    def make_my_query(value1: str, value2: int):
        # SELECT * FROM table WHERE field1 = ? AND field2 > ?
        return f'SELECT * FROM table WHERE field1 = {value1} AND field2 > {value2}'

    data = connection.execute(*make_my_query(foo, bar), [foo, bar])

Where's the injection? (you might want to take a look at the example in the issue I opened, it's more complete - I'm not bypassing the DBAPI, I'm working with it)

Yup. Your code in github explains what you're suggesting better. My bad.

It's a little difficult to parse through the README, but I believe placeholders and placeholder structure is being rendered in the SQL, not placeholder values.

    ps = f'SELECT * FROM table WHERE field1 = {q/value1} AND field2 > {q/value2}'
    # SELECT * FROM table WHERE field1 = ? AND field2 > ?

However, I would imagine that if any external input is passed through to this framework, then there might still be the possibility of SQL injection attacks passing through this framework and ending up in the prepared statement SQL.

Any string interpolation would open you up to sql injection without a lot of care. It generally recommended to used prepared statements.

I guess the q part is the query parameter for the prepared statement. I’d still be a bit antsy here given the ease of a mistake.

I just don’t get what it’s providing over using my db’s module. Usually the dialect is close but different enough to require changes to queries.

It's my biggest concern as well. For real world application I tend to use out of line filters:

    filters = [q.field == value1, q.field2 > value2]
    sql = f'SELECT * FROM table {WHERE(*filters)}'

It's a little harder to misuse.

I don’t use Python so can’t be bothered to read the source but the general solution here is to make regular string interpolation fail.

e.g. You must use the Q mechanism and doesn’t work if you forget it.

f-strings are part of the language so you need to change the interpreter to make that fail. That will not play nice with the rest of the codebase.

Reply on news.ycombinator.com