second-state / wasm-joey

Serverless Wasm - A lightweight Node.js application for deploying and executing WebAssembly(Wasm) binary-code via HTTP
https://www.secondstate.io/
Apache License 2.0
55 stars 9 forks source link

⚠️ Prevent SQL injection #32

Open katopz opened 2 years ago

katopz commented 2 years ago

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.

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

tpmccallum commented 2 years ago

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 commented 2 years ago

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)