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

Bad string generating by Expression #486

Closed
zeruk opened this issue Mar 5, 2020 · 6 comments · Fixed by #506
Closed

Bad string generating by Expression #486

zeruk opened this issue Mar 5, 2020 · 6 comments · Fixed by #506

Comments

@zeruk
Copy link

zeruk commented Mar 5, 2020

Expected Behavior

Trying to escape some random input to break expression

exp = new Influx.Expression();
exp.tag('symbol').equals.value(`GAZP()\'`).toString() // **`"symbol" = 'GAZP()\\\''`**

And it should escape characters correctly

Actual Behavior

But it doesn't escape them

exp = new Influx.Expression();
exp.tag('symbol').equals.value(`GAZP()\'`).toString() // **`"symbol" = 'GAZP()\\''`**

Steps/Code to Reproduce the Problem

const influx = require('influx')
const exp = new influx.Expression();
exp.tag('symbol').equals.value(`GAZP()\'`).toString()
// ...

Specifications

  • Version: 5.5.1
  • Platform: Darwin
@zeruk
Copy link
Author

zeruk commented Mar 5, 2020

More than that, it also allows SQL-injections:

exp.tag('symbol').equals.value(`GAZP()\' or 1=1 --`).toString()

Query with such "Where" clause will return all rows from measurement

@bencevans bencevans self-assigned this Mar 10, 2020
@stale
Copy link

stale bot commented May 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 9, 2020
@stale stale bot closed this as completed May 16, 2020
@bencevans bencevans reopened this Jun 15, 2020
@stale stale bot removed the stale label Jun 15, 2020
@bencevans bencevans removed their assignment Jun 15, 2020
@bencevans
Copy link
Member

Hi @connor4312. Hope you're well! Did you get a chance to look into this? Cheers, Ben

@connor4312
Copy link
Member

Not yet. Though I plan to use Influx again (for the first time in ages) for some home automation stuff, so I will probably be spending some time on this library again

@robertsLando
Copy link
Contributor

@bencevans any news on this? it's introducing some high security risk

bencevans added a commit that referenced this issue Jul 22, 2020
* fix: prevent sql injection #486

* refactor: grammar escape code

* fix: regex escape

* fix: eslint config

* chore: eslint fix

* fix: tring to fix regex

* fix: better test sql injection

* fix: possible solution to injection

* chore: update package-lock.json

Co-authored-by: Ben Evans <ben@bluechimp.io>
@bencevans
Copy link
Member

🎉 This issue has been resolved in version 5.5.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

4 participants