Protection against SQL injection

[thevahidal]

• Use parameterized query / prepared statement
• Specify an environment variable to restrict transaction endpoint [default (or production) disabled]
• Ideally, move all queries to the services directory

[IanMayo]

Hello @thevahidal - is Soul still secure against SQL injection?

Does the use of db.prepare protect us against SQL injection attacks?

Aah, I'm no expert at this, but it looks like better-sqlite3 would rather have the query elements passed outside the prepare statement. I believe this practice is called binding statements. Our context is a little more complex than the below example (since we have to handle lots of possible attributes), but I guess we just build up an array of items to go into prepare, and an array of items to go into all.

[thevahidal]

It's a very good question @IanMayo, I'm no expert at this too, Though here's an issue in better-sqlite about the same matter: https://github.com/WiseLibs/better-sqlite3/issues/720 Let me know if you think of a way to test it

[IanMayo]

I'm pretty sure that Soul currently fails this test:

I understand that if someone dumbly crafts a Statement without parameters for .run() and its friends, they're asking for a SQL injection attack

Note: the Soul infrastructure tightly parses incoming URLs and prevents the above issue happening.

But, in terms of layers of an onion - it seems like we should apply security at all levels where we have a chance. And providing the query params through binding statements is one extra security layer we can apply.

[thevahidal]

You're definitely right, we should start getting rid of the risky parts. I'll create a new project plan so we can prioritize our goals.

[IanMayo]

I'm sure @AbegaM and I can handle the switch to binding statements. We can get a draft PR started, for your feedback.

[thevahidal]

Thanks man, you guys are the best.

[thevahidal]

I would like to improve protection against SQL injection more by using sqlstring. I'll reopen this issue for this matter.