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

Move to a safe query structure for all the queries at contractwork.vipulnaik.com #21

Closed
vipulnaik opened this issue Jan 20, 2020 · 1 comment · Fixed by #22
Closed

Move to a safe query structure for all the queries at contractwork.vipulnaik.com #21

vipulnaik opened this issue Jan 20, 2020 · 1 comment · Fixed by #22

Comments

@vipulnaik
Copy link
Owner

@vipulnaik vipulnaik commented Jan 20, 2020

Currently, the query structure uses string interpolation to fill in values of worker, topic, payment, etc. In theory, this could allow for some kind of SQL injection into the query that could cause inappropriate SQL code to be run. I would like to move to a safer query structure that uses prepared statements. We've already been using this safer query structure in a few places, so it should be easy to do. It just needs to be applied across a bunch of files.

See also vipulnaik/donations#127

@riceissa
Copy link
Contributor

@riceissa riceissa commented Jan 21, 2020

My work is here: https://github.com/riceissa/contractwork/tree/safe-query

I think I found all the unsafe queries. All pages seem to work so this should be ready to be merged. I couldn't test the tables related to impact because those query from Wikipedia Views, which I don't have on my local machine. e.g. you might want to check https://contractwork.vipulnaik.com/format.php?format=Timeline#formatImpact

I don't have push access to this repo, so that's why it's in my own fork. Let me know if I need to send a pull request.

@vipulnaik vipulnaik mentioned this issue Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants