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

DefinedAttributeTemplateEngine parsing bug #1906

Open
kkolman opened this issue Jul 20, 2021 · 6 comments · Fixed by #2034
Open

DefinedAttributeTemplateEngine parsing bug #1906

kkolman opened this issue Jul 20, 2021 · 6 comments · Fixed by #2034
Labels

Comments

@kkolman
Copy link

kkolman commented Jul 20, 2021

Passing the following sql query

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

In SqlStatement.java

String renderedSql = statements.preparedRender(sql, ctx);

renderedSql

  • will then have string value of SELECT '\\' = '\' (single backslash on the right side of the equals sign)
  • instead of SELECT '\\' = '\\' (double backslash on the right side of the equals sign)
@macabrus
Copy link

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

@stevenschlansker
Copy link
Member

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
Copy link
Member

@kkolman @macabrus , I opened #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
Copy link
Member

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
Copy link
Member

Fixed in 3.30.0

@stevenschlansker
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants