Built-in sql escaping function needs improving

[morgo]

Bug Report

SET GLOBAL runs an SQL statement to update an internal table. The SQL that it generates does not escape string values correctly. This is not a security bug, because only SUPER users can run SET GLOBAL (can not be used to privilege escalate).

But in the future, some SET GLOBAL commands may be restricted from SUPER privilege (security enhanced mode). There might also be some sysvars introduced which require data escaped.

This is fixed in #21988 , but the function may not handle all cases:

// escape user supplied string for internal SQL. Not safe for all cases, since it doesn't
// handle quote-type, sql-mode, character set breakout.
func escapeUserString(str string) string {
	return strings.ReplaceAll(str, `'`, `\'`)
}

It is important to leave this issue open so that a more correct implementation can be added.

1. Minimal reproduce step (Required)

Take any ScopeGlobal sysvar that is of TypeString (default):

mysql> set global sync_relay_log = "'";
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 62 near "''');" 

2. What did you expect to see? (Required)

Success.

3. What did you see instead (Required)

Returns error about invalid syntax.

4. What is your TiDB version? (Required)

mysql> SELECT tidb_version()\G
*************************** 1. row ***************************
tidb_version(): Release Version: v4.0.0-beta.2-1946-geae64e40f-dirty
Edition: Community
Git Commit Hash: eae64e40fee5976dc8d22ab5ad27b3f0cdba8a67
Git Branch: infoschema-trx
UTC Build Time: 2021-01-03 19:55:38
GoVersion: go1.13.15
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false
1 row in set (0.00 sec)

[morgo]

This will be fixed in #21988 , but the escaping function should be improved to handle more cases. I will update the description of this issue to indicate.

[kennytm]

The format.OutputFormat function does proper SQL escaping when SQL_MODE does not contain NO_BACKSLASH_ESCAPES. The function escapes ', \n, \r and \0.

(I don't think it's possible to escape \n, \r and \0 with NO_BACKSLASH_ESCAPES, unless you turn _ascii'foo\r\nbar' into the hex form entirely: convert(0x666F6F0D0A626172 using ascii))

[morgo]

This was fixed in #22499