1

Suppose I have a function that looks like the following (ignore any syntax errors here unless they are relevant to the question, I'm new to SQL):

// This function updates the database using the command passed as a parameter
const execute = async (command) => {
    open({
        filename: "test.db",
        driver: sqlite3.Database,
    }).then((db) => {
        db.exec(command);
    });
};

// Takes the user ID and their input and adds it to the database
const createBlogPost = async (userId, text) => {
    await execute(`INSERT INTO posts (user_id, post) VALUES ("${userId}", "${text}");`)
}

There is nothing stopping the user from injecting their own SQL into the blog post text field. Wouldn't they be able to execute any command they want as long as the syntax is correct? I'm wondering if there's anything extra you're supposed to do in order to prevent this, or if it's best practice to just use an ORM rather than building your own SQL statements.

Many thanks.

Peter
  • 68
  • 6
  • 1
    If you're using an npm package like [sqlite](https://www.npmjs.com/package/sqlite#inserting-rows), why not use the the parameterized syntax built into it? – David784 Mar 06 '22 at 00:06
  • @David784 Thank you, seems I shouldn't have stopped reading the docs once I got the db going. Will doing so also prevent SQL injections? – Peter Mar 06 '22 at 00:21
  • 2
    If properly used, yes. – Paul T. Mar 06 '22 at 00:30
  • you can use prepared statements https://stackoverflow.com/a/49328621/17344532 – tenshi Mar 06 '22 at 01:52
  • Parameterized queries are a built in feature in every SQL vendor. Every low level driver supports them. ORMs and query builder use nothing else under the hood, since they are built on top of those drivers. A parsmertized query sends the query and parameters seperated from each other to the server. The server them evaluates the query first and fills in the params, preventing SQL injection, since it know already how the full statement looks like. – The Fool Mar 06 '22 at 13:31

1 Answers1

0

Use parameters to prevent SQL injection.

function openDb() {
    return open({
        filename: "test.db",
        driver: sqlite3.cached.Database,
    });
};

const createBlogPost = (userId, text) => {
    return openDb().then(db => db.run("INSERT INTO posts (user_id, post) VALUES (?, ?);", [userId, text]));
};

Notes:

  • An async function that does nothing but return await ... is an anti-pattern. Remove the async/await in such a case, the function will work exactly the same.

  • Running a query is very easy directly on a database object; you're actually making your life harder by trying to abstract it into a separate execute() function. That's because the database object offers a few more things than running queries, and you would need ever more wrapper functions in the long run, which is unnecessary complexity. I've created openDb(), which just directly returns a database object, that's enough.

  • I've used open() with caching for this, to prevent needlessly spamming multiple connections to the same database. This way you can call OpenDb multiple times and the existing connection is re-used.

  • You're supposed to use .run() instead of .exec() for inserting/updating rows.

  • Generally: Return the API's promises from your own functions. Here, open() gives you a promise, openDb() returns it, createBlogPost() takes it, and then also returns it to the caller. This way createBlogPost() can be awaited in calling code, and error handling works as it should, too:

    async function test_createBlogPost() {
        try {
            const result = await createBlogPost(1, 'Hello World');
            console.log(result);
        } catch (err) {
            console.log("createBlogPost failed", err);
        }
    }
    
Tomalak
  • 332,285
  • 67
  • 532
  • 628