-
-
Notifications
You must be signed in to change notification settings - Fork 99
Description
Is your feature request related to a problem? Please describe.
The approach used for the methods select_row, select_rows and update_rows is inherently flawed and vulnerable to SQL injection. In my opinion, this makes these methods unusable for all but the most trivial use cases.
Describe the solution you'd like
The solution is already partly implemented through the query_with_bindings(...)
function.
Introducing new quality of life functions providing similar functionality would be great.
The functions in question:
- select_row_with_bindings
- select_rows_with_bindings
- update_rows_with_bindings
Personally I would just let the existing functions accept a param_bindings Array and let this be the default behavior, however I understand this may not be feasible due to backward compatibility constraints.
From a quick look at the source code, it seems to me that most of the systems to achieve this are already in place.
For example in the update_rows
function in gdsqlite.cpp
, lines 584 to 593:
for (int64_t i = 0; i <= number_of_keys - 1; i++) {
query_string += (const String &)keys[i] + String("=?");
param_bindings.append(values[i]);
if (i != number_of_keys - 1) {
query_string += ", ";
}
}
query_string += " WHERE " + p_conditions + ";";
success = query_with_bindings(query_string, param_bindings);
It seems to me that appending the values of a newly introduced p_param_bindings
parameter to the existing param_bindings
variable should do the trick. This would allow us to use ?
in p_conditions
.
Again, this was just a cursory glance, I don't know the ins and outs of the code base.
Describe alternatives you've considered
It's possible to use the query_with_bindings(...)
from gdscript. This however requires us to write out the queries by hand.
Alternatively a type checker similar to the one used in query_with_bindings(...)
could be implemented in gdscript as well.