5 ms·

This is ridiculous. SQLi is a dumb problem and any development environment worth using should make it trivial to avoid. All you have to do is never allow SQL qu

This is ridiculous. SQLi is a dumb problem and any development environment worth using should make it trivial to avoid. All you have to do is never allow SQL queries to be formed using string concatenation. Or if you do, make sure you're excruciatingly strict about it. I run a large-ish web application written in ColdFusion (more precisely, CFML running on the open source Lucee) which is a language where string concatenation is happening all the time. This is a very old language which will look embarrassingly janky compared to any of the new hotness in vogue on Hacker News. But even in CFML I can trivially guarantee zero SQLi by parameterising all variables. Query parameters. They're a thing. If you don't know how your favourite programming language has implemented them, learn it. Use them.

deleted 5y ago
[deleted]
asddubs 5y ago
I agree of course, and in the case of the article you're absolutely right, although more generally there are edge cases where parameterization doesn't work, like giving choice of which row to order by. Naturally in such cases you should be operating on a whitelist of options and not pass the user input to the query, but the point is that some situations do still have to be accounted for.
- simondotau 5y ago
  I have no problem placing "if" or "switch" statements within the SQL in order to enable/disable various components of the query. The risk is when you're bringing in anything other than local constants. In the rare instances where I do this, I'm analysing the full scope of potential valid variables. I then make local copies of the variables which have been pummelled with the narrowest possible regular expression, e.g. slightly_safer = unsafe.replace(/[^0-9A-Za-z]/gi, '') Stripping away all punctuation eliminates most forms of SQLi.
  - asddubs 5y ago
    for mysql at least, if you're dealing with table names you can just do a backtick escape, where you wrap the table/column name in backticks and escape any backticks within it with two backticks. That's usually my go-to in situations like this, in addition to a whitelist. anything else you can usually parameterize
    - simondotau 5y ago
      Relying on escape sequences is a serious footgun risk as there's always going to be some weird scenario that can be exploited against you, such as alternative ways to escape backticks that quietly gets enabled when some unexpected incident forces you to rebuild your MySQL Server instance. When you're bringing in strings from outside, there's no such thing as being too paranoid. White-listing of permitted characters is the only safe approach. (And if you think your whitelist might ever need contain any kind of quote-mark, backtick or backslash, then you're wrong. Your table/identifier names are stupid and must change.)
Ciantic 5y ago
In WordPress the string concatenation in SQL is normal, even on the very first example: $results = $wpdb->get_results( "SELECT * FROM {$wpdb->prefix}options WHERE option_id = 1", OBJECT ); This is because the database tables aren't static, so one must add prefix to each table from a variable. It's horrible, it would have been far better if there was magic string like %PREFIX% to avoid that. [1]: https://developer.wordpress.org/reference/classes/wpdb/#using-the-wpdb-global-object https://developer.wordpress.org/reference/classes/wpdb/#usin...
- simondotau 5y ago
  Or even better if they didn't treat tables like confetti. I haven't analysed the wordpress schema closely but on first blush it looks like a hacky premature optimisation to solve a problem, either one of performance, or data isolation, or to avoid unravelling use of magic numbers. Realistically, content within the same "universe" should co-mingle in the same table. Multiple "universes" of content should be isolated into their own database/schema, which $wpdb should pre-select upon connection. "SELECT * FROM options WHERE prefix = ? AND option_id = 241"

deleted 5y ago

[deleted]

asddubs 5y ago

I agree of course, and in the case of the article you're absolutely right, although more generally there are edge cases where parameterization doesn't work, like giving choice of which row to order by. Naturally in such cases you should be operating on a whitelist of options and not pass the user input to the query, but the point is that some situations do still have to be accounted for.

simondotau 5y ago
I have no problem placing "if" or "switch" statements within the SQL in order to enable/disable various components of the query. The risk is when you're bringing in anything other than local constants. In the rare instances where I do this, I'm analysing the full scope of potential valid variables. I then make local copies of the variables which have been pummelled with the narrowest possible regular expression, e.g. slightly_safer = unsafe.replace(/[^0-9A-Za-z]/gi, '') Stripping away all punctuation eliminates most forms of SQLi.
- asddubs 5y ago
  for mysql at least, if you're dealing with table names you can just do a backtick escape, where you wrap the table/column name in backticks and escape any backticks within it with two backticks. That's usually my go-to in situations like this, in addition to a whitelist. anything else you can usually parameterize
  - simondotau 5y ago
    Relying on escape sequences is a serious footgun risk as there's always going to be some weird scenario that can be exploited against you, such as alternative ways to escape backticks that quietly gets enabled when some unexpected incident forces you to rebuild your MySQL Server instance. When you're bringing in strings from outside, there's no such thing as being too paranoid. White-listing of permitted characters is the only safe approach. (And if you think your whitelist might ever need contain any kind of quote-mark, backtick or backslash, then you're wrong. Your table/identifier names are stupid and must change.)

Ciantic 5y ago

In WordPress the string concatenation in SQL is normal, even on the very first example: $results = $wpdb->get_results( "SELECT * FROM {$wpdb->prefix}options WHERE option_id = 1", OBJECT ); This is because the database tables aren't static, so one must add prefix to each table from a variable. It's horrible, it would have been far better if there was magic string like %PREFIX% to avoid that. [1]: https://developer.wordpress.org/reference/classes/wpdb/#using-the-wpdb-global-object https://developer.wordpress.org/reference/classes/wpdb/#usin...

simondotau 5y ago
Or even better if they didn't treat tables like confetti. I haven't analysed the wordpress schema closely but on first blush it looks like a hacky premature optimisation to solve a problem, either one of performance, or data isolation, or to avoid unravelling use of magic numbers. Realistically, content within the same "universe" should co-mingle in the same table. Multiple "universes" of content should be isolated into their own database/schema, which $wpdb should pre-select upon connection. "SELECT * FROM options WHERE prefix = ? AND option_id = 241"