Summary:	ASTERISK-29838: ${SQL_ESC()} not correctly escaping a terminating \
Reporter:	Leandro Dardini (ldardini)	Labels:	patch security
Date Opened:	2022-01-05 14:46:18.000-0600	Date Closed:	2022-04-14 13:37:07
Priority:	Blocker	Regression?
Status:	Closed/Complete	Components:	Functions/func_odbc
Versions:	13.38.2 16.23.0 18.9.0	Frequency of Occurrence
Related Issues:
Environment:	Asterisk with MySQL realtime configuration	Attachments:	( 0) ASTERISK-29838.diff
Description:	It is possible to evade the SQL_ESC escape function by terminating the parameter by a \. The SQL_ESC function will not escape the \ and pass it directly to MySQL. If the \ is the latest character of the parameter, it is interpreted by MySQL as the escape char and the ' is not processed as field terminator. To replicate, create a simple MySQL table (for example in asterisk database): {code:title=testtable.sql} CREATE TABLE IF NOT EXISTS `testtable` (  `ID` int(11) NOT NULL AUTO_INCREMENT,  `textfield` varchar(255) COLLATE utf8_unicode_ci NOT NULL,  PRIMARY KEY (`ID`) ) {code} And the corresponding entry for writing in it in func_odbc.conf: {noformat:title=func_odb.conf} [QUERY_BUG] dsn=asterisk1,asterisk2 synopsis=Test a query bug writesql=insert into testtable(textfield) values ('${SQL_ESC(${ARG1})}') {noformat} An AEL code to trigger the problem can be written in extensions.ael as: {noformat:title=extensions.ael}        9999 => {             NoOp(This is a test);             Set(ODBC_QUERY_BUG("This is an escape ' test")="filler");             Set(ODBC_QUERY_BUG("This triggers the bug\\")="filler");             Hangup();        } {noformat} The first query will be processed correctly, escaping the ' as it should work, but the second one will trigger a SQL error. {noformat:title=asterisk full log}    -- Executing [9999@authenticated:1] NoOp("PJSIP/107-DEVEL-00000000", "This is a test") in new stack    -- Executing [9999@authenticated:2] Set("PJSIP/107-DEVEL-00000000", "ODBC_QUERY_BUG("This is an escape ' test")="filler"") in new stack [2022-01-05 21:26:40] WARNING[21524][C-00000000]: pbx_variables.c:1147 pbx_builtin_setvar: Please avoid unnecessary spaces on variables as it may lead to unexpected results ('ODBC_QUERY_BUG("This is an escape ' test")' set to '"filler"').    -- Executing [9999@authenticated:3] Set("PJSIP/107-DEVEL-00000000", "ODBC_QUERY_BUG("This triggers the bug\\")="filler"") in new stack [2022-01-05 21:26:40] WARNING[21524][C-00000000]: pbx_variables.c:1147 pbx_builtin_setvar: Please avoid unnecessary spaces on variables as it may lead to unexpected results ('ODBC_QUERY_BUG("This triggers the bug\\")' set to '"filler"'). [2022-01-05 21:26:40] WARNING[21524][C-00000000]: func_odbc.c:478 execute: SQL Execute returned an error -1: 42000: [MySQL][ODBC 8.0(a) Driver][mysqld-5.7.28-log]You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''This triggers the bug\')' at line 1 (217) [2022-01-05 21:26:40] WARNING[21524][C-00000000]: func_odbc.c:487 execute: SQL Exec Direct failed (-1)![insert into testtable(textfield) values ('This triggers the bug\')] [2022-01-05 21:26:40] WARNING[21524][C-00000000]: func_odbc.c:478 execute: SQL Execute returned an error -1: 42000: [MySQL][ODBC 8.0(a) Driver][mysqld-5.7.28-log]You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''This triggers the bug\')' at line 1 (217) [2022-01-05 21:26:40] WARNING[21524][C-00000000]: func_odbc.c:487 execute: SQL Exec Direct failed (-1)![insert into testtable(textfield) values ('This triggers the bug\')]    -- Executing [9999@authenticated:4] Hangup("PJSIP/107-DEVEL-00000000", "") in new stack {noformat} I don't know if it can be exploited leading MySQL to run different commands than expected.
Comments:	By: Asterisk Team (asteriskteam) 2022-01-05 14:46:26.242-0600 This issue has been automatically restricted and set to a blocker due to being a security type issue. If this is not a security vulnerability issue it will be moved to the appropriate issue type when triaged. Please DO NOT put a code review up for this change at this time. Attach any applicable patches to this issue. By: Asterisk Team (asteriskteam) 2022-01-05 14:46:26.298-0600 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur. Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/]. By: Joshua C. Colp (jcolp) 2022-01-05 15:45:54.580-0600 The SQL_ESC function is explicitly for escaping single ticks. It has no knowledge of \ and doesn't do anything with it. I don't think this presents any exploit because it's not allowing insertion of additional data. The worst case is causing a failed query. By: Joshua C. Colp (jcolp) 2022-01-06 10:42:26.904-0600 I should also add that while this issue is accepted, it requires further investigation to be sure that will be done in the future. I still don't believe it's a security issue on first glance, but want to be sure. By: Joshua C. Colp (jcolp) 2022-01-26 08:30:25.109-0600 I've been working my way through queries and have been unable to come up with a value that would actually allow injection or exploitation. The most I can do is cause a query failure, as is seen here. Have you given any thought to potential queries that could exploit this? By: Leandro Dardini (ldardini) 2022-01-26 15:32:51.539-0600 I have tried trying to craft a query containing two commands, but I failed like you. I was able to just crash the query By: Joshua C. Colp (jcolp) 2022-02-01 05:06:30.901-0600 Unfortunately I'm having trouble coming up with an automatic solution for this, this may end up being documentation instead about escaping '. The issue is that without parsing SQL itself, there's no way to insert the proper thing at the right time. This is because the user ultimately writes the query and is free to construct it as they wish. I could potentially also escape \ but I'm not sure of the impact of that. It's really only an issue when \ immediately preceeds ' as that is an instruction to MySQL to include that quote character within the string, which ultimately breaks the query. I'll continue giving this thought but other input is welcome too. By: Joshua C. Colp (jcolp) 2022-02-08 07:39:00.660-0600 Please try the attached patch to ensure it fixes the issue and produces the result you would expect. By: Leandro Dardini (ldardini) 2022-02-14 15:53:11.452-0600 It seems good to me. I was not able to trigger the bug anymore once patched. Really thank you By: Friendly Automation (friendly-automation) 2022-04-14 13:37:07.745-0500 Change 18374 merged by Friendly Automation: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18374|https://gerrit.asterisk.org/c/asterisk/+/18374] By: Friendly Automation (friendly-automation) 2022-04-14 13:37:54.697-0500 Change 18377 merged by Friendly Automation: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18377|https://gerrit.asterisk.org/c/asterisk/+/18377] By: Friendly Automation (friendly-automation) 2022-04-14 14:32:32.436-0500 Change 18388 merged by Michael Bradeen: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18388|https://gerrit.asterisk.org/c/asterisk/+/18388] By: Friendly Automation (friendly-automation) 2022-04-14 14:33:50.868-0500 Change 18390 merged by Michael Bradeen: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18390|https://gerrit.asterisk.org/c/asterisk/+/18390] By: Friendly Automation (friendly-automation) 2022-04-14 14:34:04.716-0500 Change 18389 merged by Michael Bradeen: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18389|https://gerrit.asterisk.org/c/asterisk/+/18389] By: Friendly Automation (friendly-automation) 2022-04-14 15:16:15.654-0500 Change 18376 merged by Michael Bradeen: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18376|https://gerrit.asterisk.org/c/asterisk/+/18376] By: Friendly Automation (friendly-automation) 2022-04-14 16:57:21.212-0500 Change 18373 merged by Joshua Colp: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18373|https://gerrit.asterisk.org/c/asterisk/+/18373] By: Friendly Automation (friendly-automation) 2022-04-14 16:57:30.647-0500 Change 18375 merged by Joshua Colp: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18375|https://gerrit.asterisk.org/c/asterisk/+/18375] By: Friendly Automation (friendly-automation) 2022-04-14 16:57:40.405-0500 Change 18372 merged by Joshua Colp: func_odbc: Add SQL_ESC_BACKSLASHES dialplan function. [https://gerrit.asterisk.org/c/asterisk/+/18372|https://gerrit.asterisk.org/c/asterisk/+/18372]