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

Preventing SQL injections and failing statements via normalized database schema #1916

Open
Alrighttt opened this issue Jul 13, 2023 · 0 comments

Comments

@Alrighttt
Copy link
Member

Whenever possible, we should be using static table names within our SQL databases. If not, it is significantly more likely we will face issues with SQL injections via the table name or unexpected failing SQL statements and therefore unexpected .unwraps() and errors.

We should aim to avoid table names influenced by user input, which includes input from various sources such as the RPC interface, configuration files, system variables, external API endpoints, and the p2p network stack.

Here is an example of how we can avoid doing this. These examples tables don't directly relate to any of the API's code.

Instead of having tables such as "KMD_blocks", "BTC_blocks", "ABC_blocks". We can do the following:

CREATE TABLE blockchain (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    name TEXT NOT NULL UNIQUE
);

CREATE TABLE block (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    blockchain_id INTEGER NOT NULL,
    height INTEGER NOT NULL,
    hash TEXT NOT NULL,
    FOREIGN KEY(blockchain_id) REFERENCES blockchain(id)
);

As we interact with the DB, we can simply select for the blockchain we need:

SELECT block.*
FROM block
JOIN blockchain ON block.blockchain_id = blockchain.id
WHERE blockchain.name = "ABC";

Obviously this does have a performance impact. I am not suggesting we absolutely must do this in every case. I have identified several bugs at this point that would have been prevented, so I feel this is worth keeping in mind.

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

1 participant