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

Replace squel package #94199

Closed
jportner opened this issue Mar 9, 2021 · 2 comments · Fixed by #97169
Closed

Replace squel package #94199

jportner opened this issue Mar 9, 2021 · 2 comments · Fixed by #97169
Labels
bug Fixes for quality problems that affect the customer experience Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@jportner
Copy link
Contributor

jportner commented Mar 9, 2021

The squel package is no longer maintained. It was last updated almost 3 years ago and is currently vulnerable to SQL Injection.

This is currently used by Canvas to create ES SQL query strings:

This dependency should be removed from Kibana.

  • The author of squel recommends knex as an alternative.
  • Another option is safe-squel, which is a fork of squel to address the SQL injection vulnerability, but it is also not maintained (see comment for details).

CC @elastic/kibana-security

@jportner jportner added bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Mar 9, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@cqliu1 cqliu1 added Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort labels Apr 13, 2021
@poffdeluxe
Copy link
Contributor

Current plan is to merge in a quick fix (#97169) to close the security hole and then follow-up and find a longer term replacement (#97170)

kueda added a commit to inaturalist/iNaturalistAPI that referenced this issue Mar 1, 2022
squel has a known vulnerability
(GHSA-4qhx-g9wp-g9m6) that doesn't technically
affect us because we don't use the relevant feature, but since it's possible
we might use it and/or forget about this vulnerability, it seems safer to
switch to safe-squel which patches the problem. This was the approach taken
by Kibana in elastic/kibana#94199. squel is not
actively maintained, and it's not clear if safe-squel is either, so it might
be better to switch to another query builder in the long term.

Note that this does change the way potential injections get handled during the
parameter interpolation we do use. Previously something like a single quote
would raise an error, and now it gets replaced with a blank string.
kueda added a commit to inaturalist/iNaturalistAPI that referenced this issue Mar 1, 2022
squel has a known vulnerability
(GHSA-4qhx-g9wp-g9m6) that doesn't technically
affect us because we don't use the relevant feature, but since it's possible
we might use it and/or forget about this vulnerability, it seems safer to
switch to safe-squel which patches the problem. This was the approach taken
by Kibana in elastic/kibana#94199. squel is not
actively maintained, and it's not clear if safe-squel is either, so it might
be better to switch to another query builder in the long term.

Note that this does change the way potential injections get handled during the
parameter interpolation we do use. Previously something like a single quote
would raise an error, and now it gets replaced with a blank string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants