r-dbi / dbi3

DBI revisited
https://r-dbi.github.io/dbi3
37 stars 2 forks source link

`dbQuoteIdentifier` should validate SQL inputs #55

Open pnacht opened 2 years ago

pnacht commented 2 years ago

Currently dbQuoteIdentifier() simply returns x if it is SQL. This is dangerous, since it is an avenue for injection.

sqlite <- DBI::dbConnect(
  RSQLite::SQLite(),
  ":memory:"
)

table <- DBI::SQL("Foo")

paste(
  "SELECT * FROM ", DBI::dbQuoteIdentifier(sqlite, table)
)
#> [1] "SELECT * FROM  Foo"

DBI::dbUnquoteIdentifier(sqlite, table)
#> [[1]]
#> <Id> table = Foo

# oh-oh!
table <- DBI::SQL("Foo; DROP TABLE Foo")

paste(
  "SELECT * FROM ", DBI::dbQuoteIdentifier(sqlite, table)
)
#> [1] "SELECT * FROM  Foo; DROP TABLE Foo"

DBI::dbUnquoteIdentifier(sqlite, table)
#> Error: Can't unquote Foo; DROP TABLE Foo

Created on 2022-03-01 by the reprex package (v2.0.1)

Note that DBI::dbUnquoteIdentifier() succeeds in the first case but errors out with the injection.

The SQL input should therefore be validated via dbUnquoteIdentifier(): if it runs, great. If it instead throws an error, then you've likely got a problem.

krlmlr commented 2 years ago

Thanks, interesting idea. This would protect users from shooting themselves in the foot, but requires that dbUnquoteIdentifier() works in all conceivable cases.