Bug #22619 Spaces in function calls leading to confusing errors
Submitted: 22 Sep 2006 23:35 Modified: 6 Dec 2006 17:27
Reporter: Marc Alff Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Parser Severity:S1 (Critical)
Version:5.1 OS:
Assigned to: Marc Alff CPU Architecture:Any

[22 Sep 2006 23:35] Marc Alff
Description:
mysql> select version();
+-----------------------+
| version()             |
+-----------------------+
| 5.1.12-beta-debug-log |
+-----------------------+
1 row in set (0.00 sec)

mysql> set sql_mode="ANSI";
Query OK, 0 rows affected (0.01 sec)

mysql> select database(), database ();
+------------+-------------+
| database() | database () |
+------------+-------------+
| NULL       | NULL        |
+------------+-------------+
1 row in set (0.00 sec)

mysql> select connection_id(), connection_id ();
+-----------------+------------------+
| connection_id() | connection_id () |
+-----------------+------------------+
|               2 |                2 |
+-----------------+------------------+
1 row in set (0.00 sec)

mysql> set sql_mode="PIPES_AS_CONCAT";
Query OK, 0 rows affected (0.00 sec)

mysql> select database(), database ();
+------------+-------------+
| database() | database () |
+------------+-------------+
| NULL       | NULL        |
+------------+-------------+
1 row in set (0.01 sec)

mysql> select connection_id();
+-----------------+
| connection_id() |
+-----------------+
|               2 |
+-----------------+
1 row in set (0.00 sec)

mysql> select connection_id ();
ERROR 1046 (3D000): No database selected

That behavior is bad.
What the parser is attempting is to find a stored function here.
Note that the function "database" works but "connection_id" does not,
since the former is implemented in the bison grammar and the later is
implemented in the LEX parser, using a CREATE_FUNC builder.

The problem is more serious that just syntax :

mysql> create database attack;
Query OK, 1 row affected (0.00 sec)

mysql> use attack;
Database changed

mysql> create function connection_id () returns varchar(255)
      ->   return "Sorry, not in the mood to connect today";
Query OK, 0 rows affected (0.00 sec)

mysql> set sql_mode=ANSI;
Query OK, 0 rows affected (0.00 sec)

mysql> select connection_id(), connection_id ();
+-----------------+------------------+
| connection_id() | connection_id () |
+-----------------+------------------+
|               3 |                3 |
+-----------------+------------------+
1 row in set (0.00 sec)

mysql> set sql_mode="PIPES_AS_CONCAT";
Query OK, 0 rows affected (0.00 sec)

mysql> select connection_id(), connection_id ();
+-----------------+-----------------------------------------+
| connection_id() | connection_id ()                        |
+-----------------+-----------------------------------------+
|               3 | Sorry, not in the mood to connect today |
+-----------------+-----------------------------------------+
1 row in set (0.00 sec)

As demonstrated, this leads to code injection.

To make things really worse, the following functions are vulnerable :

md5(), sha(), sha1()
--> can be leveraged to disable security checks in an app

aes_encrypt(), aes_decrypt(),
--> can be leveraged to access encrypted data
compress(), uncompress(), uncompress_length()
--> can be leveraged to access user data

To fix :
The way functions are handled in the LEX parser (sql/lex.h, sql_functions[])
is a design flaw that pollutes the entire language (see Bug#21114)
and happen to be also a security vulnerability.

How to repeat:
See above

Suggested fix:
Rewrite the parsing of functions.
[23 Sep 2006 12:23] Valeriy Kravchuk
Verified just as describedwith 5.1.12-BK on Linux.
[25 Sep 2006 16:56] Sergei Golubchik
I don't see a bug here. Behaviour is perfectly documented:

http://dev.mysql.com/doc/refman/5.1/en/reserved-words.html
"
You are permitted to use function names as identifiers. For example, ABS is acceptable as a column name. However, by default, no whitespace is allowed in function invocations between the function name and the following ‘(’ character. This requirement allows a function call to be distinguished from a reference to a column name.
"
http://dev.mysql.com/doc/refman/5.1/en/create-procedure.html
"
The IGNORE_SPACE SQL mode applies to built-in functions, not to stored routines. it is always allowable to have spaces after a routine name, regardless of whether IGNORE_SPACE is enabled.
"
[26 Sep 2006 21:48] Konstantin Osipov
Hello Sergey,
I'm not getting why one can conclude from the quotes you provided that this is not a bug. There is a discrepancy between behaviour of database() and connection_id () if ignore_space is not set: the parser can resolve the first function, but not the second:

 mysql> select database ();
+-------------+
| database () |
+-------------+
| test        | 
+-------------+
1 row in set (0.00 sec)

mysql> select connection_id ();
ERROR 1305 (42000): FUNCTION test.connection_id does not exist
mysql> 

I agree that not being able to find connection_id () works as expected.
[2 Oct 2006 14:59] Marc Alff
After investigation, it turns out that the possibility of code injection does not
exist, since a stored procedure code is stored in the information schema
with the SQL_MODE used at creation time.
When the storted procedure / stored function is loaded again,
the SQL_MODE of the current session has no effect on parsing.
[6 Nov 2006 17:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/14903

ChangeSet@1.2341, 2006-11-06 10:41:51-07:00, malff@weblab.(none) +8 -0
  Bug#18239 (Possible to overload internal functions with stored functions)
  Bug#21025 (misleading error message when creating functions named 'x', or 'y')
  Bug#22619 (Spaces considered harmful)
  
  This change contains a fix to report warnings or errors, and multiple tests
  cases.
  
  Before this fix, name collisions between:
  - Native functions
  - User Defined Functions
  - Stored Functions
  were not systematically reported, leading to confusing behavior.
  
  I) Native / User Defined Function
  
  Before this fix, is was possible to create a UDF named "foo", with the same
  name as a native function "foo", but it was impossible to invoke the UDF,
  since the syntax "foo()" always refer to the native function.
  After this fix, creating a UDF fails with an error if there is a name
  collision with a native function.
  
  II) Native / Stored Function
  
  Before this fix, is was possible to create a SF named "db.foo", with the same
  name as a native function "foo", but this was confusing since the syntax
  "foo()" would refer to the native function. To refer to the Stored Function,
  the user had to use the "db.foo()" syntax.
  After this fix, creating a Stored Function reports a warning if there is a
  name collision with a native function.
  
  III) User Defined Function / Stored Function
  
  Before this fix, creating a User Defined Function "foo" and a Stored Function
  "db.foo" are mutually exclusive operations. Whenever the second function is
  created, an error is reported. However, the test suite did not cover this
  behavior.
  After this fix, the  behavior is unchanged, and is now covered by test cases.
  
  Note that the code change in this patch depends on the fix for Bug 21114.
[6 Nov 2006 18:03] Marc Alff
To clarify:
After investigation, there is no security vulnerability here, which was the concern reported initially. The server behaves as intended, even if the
intended behavior is misleading.

The proposed fix only improves user experience with better messages, to avoid
confusion.

It is safe to not apply this fix.
[15 Nov 2006 2:41] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/15325

ChangeSet@1.2341, 2006-11-14 19:34:16-07:00, malff@weblab.(none) +10 -0
  Bug#18239 (Possible to overload internal functions with stored functions)
  Bug#21025 (misleading error message when creating functions named 'x', or 'y')
  Bug#22619 (Spaces considered harmful)
  
  This change contains a fix to report warnings or errors, and multiple tests
  cases.
  
  Before this fix, name collisions between:
  - Native functions
  - User Defined Functions
  - Stored Functions
  were not systematically reported, leading to confusing behavior.
  
  I) Native / User Defined Function
  
  Before this fix, is was possible to create a UDF named "foo", with the same
  name as a native function "foo", but it was impossible to invoke the UDF,
  since the syntax "foo()" always refer to the native function.
  After this fix, creating a UDF fails with an error if there is a name
  collision with a native function.
  
  II) Native / Stored Function
  
  Before this fix, is was possible to create a SF named "db.foo", with the same
  name as a native function "foo", but this was confusing since the syntax
  "foo()" would refer to the native function. To refer to the Stored Function,
  the user had to use the "db.foo()" syntax.
  After this fix, creating a Stored Function reports a warning if there is a
  name collision with a native function.
  
  III) User Defined Function / Stored Function
  
  Before this fix, creating a User Defined Function "foo" and a Stored Function
  "db.foo" are mutually exclusive operations. Whenever the second function is
  created, an error is reported. However, the test suite did not cover this
  behavior.
  After this fix, the  behavior is unchanged, and is now covered by test cases.
  
  Note that the code change in this patch depends on the fix for Bug 21114.
[15 Nov 2006 8:53] Konstantin Osipov
Code review done by email.
[30 Nov 2006 1:35] Konstantin Osipov
Pushed into 5.1.13
[6 Dec 2006 17:27] Paul Dubois
Noted in 5.1.14 changelog.

Incompatible change: Previously, you could create a user-defined
function (UDF) or stored function with the same name as a built-in
function, but could not invoke the UDF. Now an error occurs if you
try to create such a UDF. The server also now generates a warning if 
you create a stored function with the same name as a built-in
function. It is not considered an error to create a stored function
with the same name as a built-in function because you can invoke the
function using db_name.func_name() syntax. However, the server now
generates a warning in this case.

I have also written a new section for the manual describing the
rules for function name resolution. It will appear here:

http://dev.mysql.com/doc/refman/5.1/en/function-resolution.html