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.

CodePudding user response：

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);
      }
  }