-
Notifications
You must be signed in to change notification settings - Fork 77
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
Why does SqlString.escapeString return values in single quotes? #19
Comments
Hi! The reason the string are surrounded in quotes is such that it formed an escaped value for insertion into SQL. For example, here is a typical SQL statement: SELECT * FROM user WHERE username = ? AND password_hash = ? And then you can use this module to escape in the two strings: var sql = SqlString.format('SELECT * FROM user WHERE username = ? AND password_hash = ?', [username, passwordHash])
// => SELECT * FROM user WHERE username = 'admin' AND password_hash = X'5f4dcc3b5aa765d61d8327deb882cf99' In addition, it also prevents the common SQL injection of just including a space character inside a string. For example, the username "admin' OR 1=1 --" would, without quoting, become the following: var sql = SqlString.format("SELECT * FROM user WHERE username = '?' AND password_hash = ?", [username, passwordHash])
// => SELECT * FROM user WHERE username = 'admin' OR 1=1 --' AND password_hash = X'5f4dcc3b5aa765d61d8327deb882cf99'
I hope that helps! |
Ah, I didn't realize that syntax was available. I understand why the single quotes are added to the SQL query. What I still don't understand though is why string values are returned with single quotes. |
Why does it even matter? The escapeString function is private and not even exported, so you cannot even use it. What is the public function you are using, and I can likely provide a better answer. |
I'm going to close since I haven't heard back in 2 weeks. If you let me know what public function you're using, I'd be happy to provide a better answer. |
(@ManBearPigg deleted the post above, so here is the original comment quoted that I am replying to)
Hi @ManBearPigg yes, that is because the var sql = 'SELECT * FROM user WHERE first_name = ' + SqlString.escape(name) This will create a valid SQL string given If the quotes are removed, then not only will invalid SQL get produced, but then SQL injection attacks will happen. I hope that explains why the quotes are added -- you have to quote strings in SQL. I'm not really sure why you would be creating code as in your example, because none of that is even SQL, which is what the |
That was not SQL because it was just a fast example. So you're saying that in order to prevent SQL injection, it is expected that any string inserted into my db will have single quotes added? As in var name = "'manbearpigg'" ? (edit: omg github won't let me demonstrate the escape backslash. I meant "[backslash]'manbearpigg[backslash]'"). Because that is the result I'm getting. Even when I select string values from my DB, which I inserted with my hacked fork so as not to add single quotes, they are selected with single quotes added when I get them out of the DB. Here is an example of how I was using it: var query = If I SELECT from my own terminal, I find username = Jake or whatever. So yeah, it's altering my strings. It's basically adding escaped single quotes to all of my strings. Is that expected behaviour designed to prevent SQL injection? Or am I doing this wrong? |
Hi @ManBearPigg yea, there sounds like there is something else going on. Would it be possible to provide full code that I can run to see this happening and debug? I'd be happy to figure out how this is happening in your code -- this must be some case where |
I figured it out. I was actually doing Edit: I think that github removing some of these quotes has contributed to the confusion in this conversation lol I guess that mysql.escape() adds the quotes for you so that people can just use the question mark notation. So I'm pretty sure since I added quotes, then mysql.escape added quotes, the outer quotes "" were saying "this is a string" and the inner quotes created by the function '' were saying "this is a string" but at that point it was redundant so they got added as if they were part of the string itself. Removing the double quotes around mysql.escape() seems to have fixed the problem. Thanks for your support. :D |
Ah, that makes sense. Yea, since it was surrounded in the double quotes it ended up where the single quotes became literals in the value instead of being the quotes. Yea, it adds quotes for a couple helpful reasons:
I believe both those two issues have plagued the old PHP escaping functions in the past, which has bene the main inspiration to quoting the values within the escaped values to remove a foot gun that PHP users would routinely run into. I hope that makes sense and I'm glad you were able to resolve the issue :) ! |
For example, when I use this library to escape a username I would like to have the username returned in the form Jake as opposed to 'Jake'. What are the security reasons for returning strings with single quotes in this way?
The text was updated successfully, but these errors were encountered: