jdbi / jdbi

The Jdbi library provides convenient, idiomatic access to relational databases in Java and other JVM technologies such as Kotlin, Clojure or Scala.
http://jdbi.org/
Apache License 2.0
1.98k stars 341 forks source link

DefinedAttributeTemplateEngine parsing bug #1906

Open kkolman opened 3 years ago

kkolman commented 3 years ago

Passing the following sql query

String sql = "SELECT '\\\\' = '\\\\'";
Handle handle = jdbi.open());
       handle.createQuery(sql)

In SqlStatement.java https://github.com/jdbi/jdbi/blob/34a81e7cdd7ced4156aebf9708d084f68a9b6e04/core/src/main/java/org/jdbi/v3/core/statement/SqlStatement.java#L1806

renderedSql

macabrus commented 2 years ago

This looks serious, has it been fixed? Is DefinedAttributeTemplateEngine used by default? If so, is my application vulnerable to injections?

stevenschlansker commented 2 years ago

Unfortunately no. I'd happily review any community contributions to fix it, otherwise it's on my radar to spend some time fixing soon. I'm not quite sure how difficult the fix is.

Unfortunately, it's also hard for me to evaluate whether it makes your application vulnerable to injection. But I do agree this is important to fix.

stevenschlansker commented 2 years ago

@kkolman @macabrus , I opened https://github.com/jdbi/jdbi/pull/2034 which should fix this. Would either of you be able to build jdbi and verify that it fixes the issue for you? We'll include it in the next release. Thanks!

stevenschlansker commented 2 years ago

Also, I don't think there is much security risk - you already should not be interpolating user provided data into template strings, but instead bind them as parameters. As long as all user provided data is properly bound as parameters there should be minimal risk.

stevenschlansker commented 2 years ago

Fixed in 3.30.0

stevenschlansker commented 2 years ago

This fix was reverted due to breakage reported in #2084 :(