Do I need to prepare my queries when there's number input only?

Viewed 36

I have some JS apps associated with MySQL. I have limited knowledge of MySQL, but I know that I mustn't send the raw user input directly to DB, so I prepared the queries using ? like the following code, according to npm js's preparing queries.

connection.query('SELECT * FROM users WHERE id = ?', [userId], function (error, results, fields) {
  if (error) throw error;
  // ...
});

connection.query('UPDATE users SET foo = ?, bar = ?, baz = ? WHERE id = ?', ['a', 'b', 'c', userId], function (error, results, fields) {
  if (error) throw error;
  // ...
});

Other apps with these prepared queries work fine. (I'm not sure it will be enough though)

But I have this app, taking the order from user input(there's no keyboard input), and sending them to DB. So the only input from the user is numbers. My app will be accessible for anyone, and I granted the user SELECT, INSERT, DELETE. Should I sanitize/prepare queries for this case also? If so, what method should I use to do so? The above way is not working for these queries.

enter image description here


//When the user puts items in the cart, bring the equivalent product info by joining them with product info table
router.post("/", (req, res) => {
    const bodyData = req.body;
    db.query(
        `INSERT INTO cart_table (item_id) VALUES(${bodyData.id})`, (err, rows) => {
            db.query(
                `SELECT cart_table.id as order_id, item_name, item_price, item_category, count, product_info.id as id FROM product_info JOIN cart_table on product_info.id = cart_table.item_id where item_id = ${bodyData.id}`,
                (err, orderedItem) => {
                    res.json({ 'orderedItem': orderedItem[0] });
                }
            );
        }
    );
});

// When user clicks an item, add the selected item or increase selected item's quantity in cart table
router.put("/", (req, res) => {
    const bodyData = req.body;
    db.query(
        `UPDATE cart_table SET count = ${bodyData.count ? parseInt(bodyData.count) : "count+1"
        } WHERE item_id = ${bodyData.item_id}`,
        (err, rows) => {
            res.send({ status: 200 });
        }
    );
});

// When the user orders, insert cart items to the order table and empty the cart table.
router.post("/", (req, res) => {
    const bodyData = req.body;
    const values = bodyData.map(({count, id}) => `(${count === null ? 1 : count}, ${id})`).join(",");
    db.query(
        `INSERT INTO order_table (count, item_id) VALUES${values}`,
        (err, rows) => {
            const order_id_array = bodyData.map(({ order_id }) => `id = ${order_id}`).join(" or ");
            db.query(
                `DELETE FROM cart_table`
            );
            res.send({ status: 200 });
        }
    );
});

1 Answers

Http requests can be spoofed. That is, if a form can send the request from a browser, then anyone can use curl or another http client to create a POST request with a different request body. So you can't count on id being an integer.

Just use the prepared query and make the value a parameter, and you won't have to wonder if it's safe. It will be.

It's worth forming a habit of using prepared statements all the time when you combine variables with your SQL statements. It reduces the possibility of mistakes. It makes your code more readable.

Keep in mind that in a professional company, many times you write code and then months or years later some other person will take over responsibility for maintaining it. You should make your code as clear as possible, and do not avoid best practices or write code that is too clever. The people who have to support your code will speak your name with respect and gratitude.

Related