3 ms·

Both are true. When the request input is blatantly the wrong type like this, you shouldn't even be attempting to execute the sql query. But of course JavaScript

by Merad 5y ago

Both are true. When the request input is blatantly the wrong type like this, you shouldn't even be attempting to execute the sql query. But of course JavaScript encourages developers to be lazy and not even think about such things.

ovex 5y ago
The problem is not only that types are not checked but that strings are simply replaced in structured input. Without having looked in detail, I would bet that the implementation also fails at queries which contain question marks inside strings, i.e. question marks that are not placeholders. Even if string escaping was the right way, which it is not, the proper way to do it would be to translate the SQL statement into an AST and then replace the leaves of the tree that are placeholders with the respective escaped strings.
hn_throwaway_99 5y ago
Developers shouldn't have to think about these things. If I'm doing any sort of string interpolation, whether it be in a SQL statement like this or in something like a log or print formatter, I should be able to put whatever the f I want into the value, without checking it. That's the whole point of using interpolated statements like this in the first place: it should be safe to run (but may throw something like a type error in the DB) regardless of what is stuffed in the value. This is solely a mysqljs bug.
- Merad 5y ago
  If you use any strongly typed language with a halfway decent framework you _don't_ have to think about this because the request will automatically be validated and rejected for invalid input before your code is ever hit.
  - nicoburns 5y ago
    That’s also true of a decent JS library. The problem here being that the library wasn’t decent. A library in a strongly typed language would still need to use parameterised queries or escape the strings just like a JS library. And it would be just as easy or difficult to have an incorrect sanitising function. In fact, the specified string is not invalid input, it would be perfectly valid to store backticks in a string field in MySQL. They just need to be correctly escaped before being submitted to the database.
  - hn_throwaway_99 5y ago
    That's not really true. Pretty much every DB driver I know, in every language, will take prepared statement values that are any valid DB datatype: strings, ints, decimals, etc., and, importantly, JSON values because most DBs now support JSON. Many JS DB frameworks will take any JS object and convert it to a JSON string because that's obviously a trivial operation (it is "JavaScript Object Notation" after all). Again, the only bug here is the completely incorrect serialization that mysqljs does.
    - Merad 5y ago
      You're missing my point because you're focused on the database query. I'm talking about the web application framework. If this was written in Asp.Net, for example, the password field would be declared as a string and the data binding code within Asp.Net would validate that the field was in fact a string as part of instantiating a model for the strongly typed request body. If someone passed in an object for that field your code for the endpoint, including all of the database interaction, would never be called because the framework would automatically return a 400 response when data binding failed.
- thinkharderdev 5y ago
  I don't think it would actually be possible to use proper prepared statements for a "feature" like this. The whole point is that query is built dynamically from any JSON object. So you could, strictly speaking, build a prepared statement dynamically but you would have the exact same bug.
zamalek 5y ago
Nope. Primarily, this stance is hostile to international users. We have all the tools required to transparently move data from the front-end to the backend, and back to the UI. Furthermore, simply because my password begins with '{', contains '{', or is 4367 characters long shouldn't be a concern whatsoever. Sanitization is an ignorantly dangerous practice, because it obscures much safer practices. Validation? That's for business rules. No more order items than there are things in stock, and stuff like that. If you need to do that because something in your stack needs it, then it's far past the time to reconsider your stack.

by Merad 5y ago

ovex 5y ago

The problem is not only that types are not checked but that strings are simply replaced in structured input. Without having looked in detail, I would bet that the implementation also fails at queries which contain question marks inside strings, i.e. question marks that are not placeholders. Even if string escaping was the right way, which it is not, the proper way to do it would be to translate the SQL statement into an AST and then replace the leaves of the tree that are placeholders with the respective escaped strings.

hn_throwaway_99 5y ago

Developers shouldn't have to think about these things. If I'm doing any sort of string interpolation, whether it be in a SQL statement like this or in something like a log or print formatter, I should be able to put whatever the f I want into the value, without checking it. That's the whole point of using interpolated statements like this in the first place: it should be safe to run (but may throw something like a type error in the DB) regardless of what is stuffed in the value. This is solely a mysqljs bug.

Merad 5y ago
If you use any strongly typed language with a halfway decent framework you _don't_ have to think about this because the request will automatically be validated and rejected for invalid input before your code is ever hit.
- nicoburns 5y ago
  That’s also true of a decent JS library. The problem here being that the library wasn’t decent. A library in a strongly typed language would still need to use parameterised queries or escape the strings just like a JS library. And it would be just as easy or difficult to have an incorrect sanitising function. In fact, the specified string is not invalid input, it would be perfectly valid to store backticks in a string field in MySQL. They just need to be correctly escaped before being submitted to the database.
- hn_throwaway_99 5y ago
  That's not really true. Pretty much every DB driver I know, in every language, will take prepared statement values that are any valid DB datatype: strings, ints, decimals, etc., and, importantly, JSON values because most DBs now support JSON. Many JS DB frameworks will take any JS object and convert it to a JSON string because that's obviously a trivial operation (it is "JavaScript Object Notation" after all). Again, the only bug here is the completely incorrect serialization that mysqljs does.
  - Merad 5y ago
    You're missing my point because you're focused on the database query. I'm talking about the web application framework. If this was written in Asp.Net, for example, the password field would be declared as a string and the data binding code within Asp.Net would validate that the field was in fact a string as part of instantiating a model for the strongly typed request body. If someone passed in an object for that field your code for the endpoint, including all of the database interaction, would never be called because the framework would automatically return a 400 response when data binding failed.
thinkharderdev 5y ago
I don't think it would actually be possible to use proper prepared statements for a "feature" like this. The whole point is that query is built dynamically from any JSON object. So you could, strictly speaking, build a prepared statement dynamically but you would have the exact same bug.

zamalek 5y ago

Nope. Primarily, this stance is hostile to international users. We have all the tools required to transparently move data from the front-end to the backend, and back to the UI. Furthermore, simply because my password begins with '{', contains '{', or is 4367 characters long shouldn't be a concern whatsoever. Sanitization is an ignorantly dangerous practice, because it obscures much safer practices. Validation? That's for business rules. No more order items than there are things in stock, and stuff like that. If you need to do that because something in your stack needs it, then it's far past the time to reconsider your stack.