Skip to content
This repository was archived by the owner on Mar 10, 2025. It is now read-only.

⚠️ Prevent SQL injection #32

Open
katopz opened this issue Jan 4, 2022 · 2 comments
Open

⚠️ Prevent SQL injection #32

katopz opened this issue Jan 4, 2022 · 2 comments

Comments

@katopz
Copy link

katopz commented Jan 4, 2022

Is your feature request related to a problem? Please describe.
I think current SQL code didn't escape or prevent SQL injection? e.g.

var sqlSelect = "SELECT wasm_binary from wasm_executables WHERE wasm_id = '" + _wasm_id + "';";

Describe the solution you'd like
We should prevent SQL injection somehow.

  • placeholders
  • named placeholders
  • node-mysql module

as explain here https://blog.sqreen.com/preventing-sql-injection-in-node-js-and-other-vulnerabilities/

@tpmccallum
Copy link
Contributor

Thanks @katopz
wasm-joey has a single function which performs the SQL request so I think the best way to implement escaping is to use the generic ? placeholder (and pass in both the query string and an array of the parameters to map).

function performSqlQuery(string_query, parameter_array) {
    return new Promise(function(resolve, reject) {
        connection.query(string_query, parameter_array, function(err, resultSelect) {
            if (err) {
                res.status(400).send("Perhaps a bad request, or database is not running");
            }
            resolve(resultSelect);
        });
    });
}

The code calling this single performSqlQuery would then be updated to look like the following.

function updateAOT(_wasm_id, _ssvm_options, _is_an_update) {
    // snip
    var sqlSelect = "SELECT wasm_binary from wasm_executables WHERE wasm_id =  ?;";
    var parameterArray = [_wasm_id];
    performSqlQuery(sqlSelect, parameterArray).then((result, error) => {
        // snip
    });
}

@tpmccallum
Copy link
Contributor

tpmccallum commented Apr 1, 2022

Updates have started on a new branch https://github.com/second-state/wasm-joey/tree/update_sql_as_per_issue_1

Placeholders have been implemented up until line 1563 and will continue to be updated from line 1592 onwards (approximately 19 of 52 have been updated at this stage)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants