On Wed, 11 May 2022 at 07:52, Rainer Deyke via Boost
On 10.05.22 16:59, Rainer Deyke via Boost wrote:
On 10.05.22 09:14, Richard Hodges via Boost wrote:
The Boost formal review of the MySQL library starts Today, taking place from May 10th, 2022 to May 19th, 2022 (inclusive) - We are starting one day after the announced date and extending the period by one day to compensate. I took a quick look, and my first impression is that the library doesn't do enough to prevent SQL injection attacks. Yes, text queries are convenient when the full query is known at compile-time. Yes, security is ultimately the responsibility of those who use the API. Yes, this is C++, where far worse security flaws are a constant threat. Even so, connection::query gives me shivers.
So, I've been thinking about what the library can do to prevent SQL injection attacks. Ideas: - connection::query can be renamed to connection::unsafe_query.
That reminds me of React naming. It inspires me "don't use this function unless you're really knowledgeable of the library's internals and you know what you are doing". I don't think that's the case. For example, if you're using transactions, you'll be calling `conn.query("COMMIT")`. I'm not very keen on making this `conn.unsafe_query("COMMIT")`.
- A big red warning about SQL injection attacks can be added to the documentation.
I've raised https://github.com/anarthal/mysql/issues/66 to address it.
- Syntax sugar for a one-off parametrized query wouldn't hurt either.
This requires a decent amount of work, as it requires implementing SQL sanitizing client-side. I'm not very keen on it, as it's very possible to get it wrong and end up introducing a vulnerability that wouldn't have existed with prepared statements. I can have a look at how complex would this be if the community thinks it really adds a lot of value.
- As a nuclear option, the query string can be changed into a template argument to prevent its use with strings that aren't known at compile-time. Unfortunately this would also prevent some valid uses of connection::query.
As you already say, I think it's too nuclear - this is C++ and I think we should treat our users as adults.
If I were to design an API, I might use a combination of the above:
// Allowed: con.query("SELECT * FROM employee WHERE company_id = ?", "HGS"); con.query<"SELECT * FROM employee WHERE company_id = 'HGS'">(); con.unsafe_query("SELECT * FROM employee WHERE company_id = 'HGS'");
// Error: literals in runtime-supplied query string not allowed. con.query("SELECT * FROM employee WHERE company_id = 'HGS'");
But I'm just throwing out ideas random here.
-- Rainer Deyke (rainerd@eldwood.com)
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost