| 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: | |
| Category: | MySQL Server: Parser | Severity: | S1 (Critical) |
| Version: | 5.1 | OS: | |
| Assigned to: | Marc ALFF | CPU Architecture: | Any |
[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
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.