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

Dangerous SQL injection #37

Open
oliverjam opened this issue May 20, 2022 · 1 comment
Open

Dangerous SQL injection #37

oliverjam opened this issue May 20, 2022 · 1 comment

Comments

@oliverjam
Copy link

function selectPet(filter) {
const selectPets = /* sql */ `
SELECT pets.id, pets.pet_name, pets.birth_date, pet_type.pet_kind
FROM pets
INNER JOIN pet_type ON pets.type_id = pet_type.id
WHERE ${filter};
`;
return db.query(selectPets).then((result) => result.rows)
}

It's important to never insert user input into a DB query. This WHERE clause is coming directly from the user:

function get(request, response) {
const filterTerm = request.query.type;
let filterType = "";
if (filterTerm && (filterTerm != 0)) {
filterType = `pet_type.id = ${filterTerm.replaceAll("<", "&lt;")}`;
} else {
filterType = "1=1";
}

This means a user can easily execute arbitray SQL code on your server, giving them the ability to read other users' data, or even just delete everything. The fact you're using a <select> for this value unfortunately doesn't help, since it's easy for a user to bypass client-side protection like that and submit anything they want. E.g. they could edit one of the <option>s to change the value to something like 1=1;DROP TABLE users;

It is very important to only ever use paramterized queries for this (i.e. pass user values as an array in the second argument to .query)

@Glombort
Copy link
Contributor

Oh nice, I assumed the paramaterized queries was for INSERT only, but this makes a lot of sense for any user input and should be an easy fix:))

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

No branches or pull requests

2 participants