-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle escaping for different SQL_MODE parameters #1828
Comments
Pull requests for any of the options are certainly welcome! |
Created pull request 1830. This is just a warning in the ReadMe. I may try to implement something. Ideally this would be a configuration on the DB connection, but
From then on, any calls to What do you think of this idea? I don't want to go through the effort of learning your code and coding it if this is just going to be rejected. :) If there is another approach you would prefer please let me know. |
It's possible, but then you'll be in a similar (or worse?) situation if you are connecting to two servers in the same process where one has NO_BACKSLASH_ESCAPES and one does not. |
Yes I thought about that too. In my experience that would be uncommon, but maybe for someone else it is not. I just noticed in re-reading your docs that there is also a
|
Without telemetry, it's impossible to know what is common and what is not; best just to support as much as possible. I think that makes sense. I'm not very familiar with the NO_BACKSLASH_ESCAPES and didn't find a section on how things like |
In fact, it looks like the server will indicate in the initial handshake if it is in NO_BACKSLASH_ESCAPES mode, so I would say use that from the server to set an internal flag instead of requiring the user to specify it as a connection option so the handling is always correct. |
Hmm okay. I don't think I've ever encountered those kinds of characters in strings. My company uses the flag because of porting from SQL Server to MySQL some years back, and the no backslash mode makes it behave like SQL Server. Using the handshake value might actually break the code for anyone using the same workaround as me: In my production code I'm calling |
Ah, yea. So that would mean that after every query the status flags from the OK packet should be checked for the NO_BACKSLASH_ESCAPES and update as necessary, which would automatically account for that I believe. Re chars in string, in order to implement escapes without backslashes, probably need to understand what the escaping rules actually are. I think the rules for |
Why are you escaping
|
That's how the module was before I was here, so don't have an answer for you on the why. |
mysql.escape()
and prepared statements do not escape strings properly with different SQL_MODE parameters are used.My company has our MySQL database default to NO_BACKSLASH_ESCAPES mode. In this mode
'
must be escaped as''
, rather than\'
. We have found out (the hard way) that this library always assumes MySQL is operating in standard mode.This is especially surprising in prepared statements (or what appear to be prepared statements but just borrow the syntax):
In other languages, code like this doesn't need to care what SQL_MODE is set to, the DB driver handles that for you, you just parameterize your queries and you're protected from SQL Injection.
As a workaround, I'm now running
SET SESSION SQL_MODE = ''
immediately after creating the DB connection. But it would be much better if this were somehow incorporated as an option. Either a global config for the mysql object, or a config option on the DB connection. Or at the very least there should be a big warning somewhere in the documentation.Example of a potential vulnerability:
In this case, query is executed like:
In NO_BACKSLASH_ESCAPES mode, that means: where username is a backslash or userid=17. Assuming there's no user named backslash, but userid 17 is somehow interesting, then you've exposed data.
The text was updated successfully, but these errors were encountered: