Skip to content
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

[core] Escaping values for SQL in script raw querys #1530

Open
ar45 opened this issue Nov 13, 2018 · 6 comments
Open

[core] Escaping values for SQL in script raw querys #1530

ar45 opened this issue Nov 13, 2018 · 6 comments
Assignees

Comments

@ar45
Copy link
Contributor

ar45 commented Nov 13, 2018

The SQL standard way to escape single quotes is by doubling them, and most databases do not recognize \ as an escape character. And thus aren't properly escaped by postgres leaving queries vulnerable to SQL injections.

From postgresql docs https://www.postgresql.org/docs/current/runtime-config-compatible.html

backslash_quote (enum)
This controls whether a quote mark can be represented by ' in a string literal. The preferred, SQL-standard way to represent a quote mark is by doubling it ('') but PostgreSQL has historically also accepted '. However, use of ' creates security risks because in some client character set encodings, there are multibyte characters in which the last byte is numerically equivalent to ASCII . If client-side code does escaping incorrectly then a SQL-injection attack is possible. This risk can be prevented by making the server reject queries in which a quote mark appears to be escaped by a backslash. The allowed values of backslash_quote are on (allow ' always), off (reject always), and safe_encoding (allow only if client encoding does not allow ASCII \ within a multibyte character). safe_encoding is the default setting.

Note that in a standard-conforming string literal, \ just means \ anyway. This parameter only affects the handling of non-standard-conforming literals, including escape string syntax (E'...').

So the only way to have strings containing ' escaped is by prefixing them with E'...'

@bogdan-iancu
Copy link
Member

Hi @ar45 - the actual purpose of {s.escape.common} is fuzzy if you ask me, as it is not affiliated to any clear escaping policy. It does \ based escaping , but only for ', ", \ and 0 .
The mysql escaping works with \ - see https://dev.mysql.com/doc/refman/8.0/en/string-literals.html, but also by doubling the single quotes (not sure about double quotes?!).
Is there any SQL (accepted by all(most) implementations) in terms of quoting ?

@bogdan-iancu
Copy link
Member

OK, some update here. After some research, it seems that the single quote is the standard SQL way of quoting strings. Escaping them is by doubling, indeed (and not with '').
While the standard/formated queries are safe in opensips (as each DB backend is doing it's own escaping when packing the strings into the query), the raw queries from script are not safe, so you need to do standard SQL escaping (doubling the single quotes) - of course, this means you need to format your query by packing the string with single quote.
Or many an {s.sql_string} transformation that will escape all the inner single quotes and also add the wrapping quotes too.

@bogdan-iancu bogdan-iancu self-assigned this Jan 4, 2019
@bogdan-iancu bogdan-iancu changed the title Escaping values for SQL using escape.common is not safe by default [core] Escaping values for SQL in script raw querys Jan 4, 2019
@ar45
Copy link
Contributor Author

ar45 commented Jan 4, 2019 via email

@bogdan-iancu
Copy link
Member

Hi @ar45,
That's an interesting idea, but what what about drivers with out such escaping function?
As the effort of doing this will be not small (as the DB API must be extended to export the escaping function), out of my curiosity, what are these escaping function (like the mysql and postgres) more than doubling the single quotes ?
I'm asking as maybe there is no pay off for the effort of using the driver native escaping, versus simply escaping the single quotes.

@ar45
Copy link
Contributor Author

ar45 commented Jan 21, 2019

These functions do the escaping while being aware of the database character encoding.

https://security.stackexchange.com/a/9910

@stale
Copy link

stale bot commented Jun 18, 2019

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants