From 9af756efd309720597962519f28c0f5ab62d1d22 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@alik." <> Date: Wed, 23 Aug 2006 21:31:00 +0400 Subject: [PATCH] Fix for BUG#16899: Possible buffer overflow in handling of DEFINER-clause User name (host name) has limit on length. The server code relies on these limits when storing the names. The problem was that sometimes these limits were not checked properly, so that could lead to buffer overflow. The fix is to check length of user/host name in parser and if string is too long, throw an error. --- BitKeeper/etc/collapsed | 1 + mysql-test/r/grant.result | 24 ++++++++++++++++++ mysql-test/r/sp.result | 13 ++++++++++ mysql-test/r/trigger.result | 13 ++++++++++ mysql-test/r/view.result | 11 +++++++++ mysql-test/t/grant.test | 49 +++++++++++++++++++++++++++++++++++++ mysql-test/t/sp.test | 26 ++++++++++++++++++++ mysql-test/t/trigger.test | 30 +++++++++++++++++++++++ mysql-test/t/view.test | 27 ++++++++++++++++++++ sql/mysql_priv.h | 1 + sql/share/errmsg.txt | 6 +++++ sql/sql_acl.cc | 33 ------------------------- sql/sql_parse.cc | 38 ++++++++++++++++++++-------- sql/sql_yacc.yy | 18 ++++++++------ 14 files changed, 239 insertions(+), 51 deletions(-) diff --git a/BitKeeper/etc/collapsed b/BitKeeper/etc/collapsed index 8b90deae622..c008b9f18fc 100644 --- a/BitKeeper/etc/collapsed +++ b/BitKeeper/etc/collapsed @@ -1 +1,2 @@ 44d03f27qNdqJmARzBoP3Is_cN5e0w +44ec850ac2k4y2Omgr92GiWPBAVKGQ diff --git a/mysql-test/r/grant.result b/mysql-test/r/grant.result index 3f3325354ee..e755822c490 100644 --- a/mysql-test/r/grant.result +++ b/mysql-test/r/grant.result @@ -867,3 +867,27 @@ insert into mysql.user select * from t2; flush privileges; drop table t2; drop table t1; +GRANT CREATE ON mysqltest.* TO 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +GRANT CREATE ON mysqltest.* TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +REVOKE CREATE ON mysqltest.* FROM 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +REVOKE CREATE ON mysqltest.* FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +GRANT CREATE ON t1 TO 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +GRANT CREATE ON t1 TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +REVOKE CREATE ON t1 FROM 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +REVOKE CREATE ON t1 FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +GRANT EXECUTE ON PROCEDURE p1 TO 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +GRANT EXECUTE ON PROCEDURE p1 TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +REVOKE EXECUTE ON PROCEDURE p1 FROM 1234567890abcdefGHIKL@localhost; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +REVOKE EXECUTE ON PROCEDURE t1 FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 7440ee16007..d4c77bc47e5 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -5374,4 +5374,17 @@ a 1 use test| drop table t3| +DROP PROCEDURE IF EXISTS bug16899_p1| +DROP FUNCTION IF EXISTS bug16899_f1| +CREATE DEFINER=1234567890abcdefGHIKL@localhost PROCEDURE bug16899_p1() +BEGIN +SET @a = 1; +END| +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY +FUNCTION bug16899_f1() RETURNS INT +BEGIN +RETURN 1; +END| +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) drop table t1,t2; diff --git a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result index f3e797d2344..b41dd66c390 100644 --- a/mysql-test/r/trigger.result +++ b/mysql-test/r/trigger.result @@ -1089,4 +1089,17 @@ begin set @a:= 1; end| ERROR HY000: Triggers can not be created on system tables +use test| +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +CREATE TABLE t1(c INT); +CREATE TABLE t2(c INT); +CREATE DEFINER=1234567890abcdefGHIKL@localhost +TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a = 1; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY +TRIGGER t2_bi BEFORE INSERT ON t2 FOR EACH ROW SET @a = 2; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +DROP TABLE t1; +DROP TABLE t2; End of 5.0 tests diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 534065a33b6..c10f7d49157 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2850,3 +2850,14 @@ Tables_in_test t1 DROP TABLE t1; DROP VIEW IF EXISTS v1; +DROP TABLE IF EXISTS t1; +DROP VIEW IF EXISTS v1; +DROP VIEW IF EXISTS v2; +CREATE TABLE t1(a INT, b INT); +CREATE DEFINER=1234567890abcdefGHIKL@localhost +VIEW v1 AS SELECT a FROM t1; +ERROR HY000: String '1234567890abcdefGHIKL' is too long for user name (should be no longer than 16) +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY +VIEW v2 AS SELECT b FROM t1; +ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY' is too long for host name (should be no longer than 60) +DROP TABLE t1; diff --git a/mysql-test/t/grant.test b/mysql-test/t/grant.test index a9d52f559ca..94b63389771 100644 --- a/mysql-test/t/grant.test +++ b/mysql-test/t/grant.test @@ -681,3 +681,52 @@ drop table t2; drop table t1; +# +# Test for BUG#16899: Possible buffer overflow in handling of DEFINER-clause. +# +# These checks are intended to ensure that appropriate errors are risen when +# illegal user name or hostname is specified in user-clause of GRANT/REVOKE +# statements. +# + +# Working with database-level privileges. + +--error ER_WRONG_STRING_LENGTH +GRANT CREATE ON mysqltest.* TO 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +GRANT CREATE ON mysqltest.* TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; + +--error ER_WRONG_STRING_LENGTH +REVOKE CREATE ON mysqltest.* FROM 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +REVOKE CREATE ON mysqltest.* FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; + +# Working with table-level privileges. + +--error ER_WRONG_STRING_LENGTH +GRANT CREATE ON t1 TO 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +GRANT CREATE ON t1 TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; + +--error ER_WRONG_STRING_LENGTH +REVOKE CREATE ON t1 FROM 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +REVOKE CREATE ON t1 FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; + +# Working with routine-level privileges. + +--error ER_WRONG_STRING_LENGTH +GRANT EXECUTE ON PROCEDURE p1 TO 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +GRANT EXECUTE ON PROCEDURE p1 TO some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; + +--error ER_WRONG_STRING_LENGTH +REVOKE EXECUTE ON PROCEDURE p1 FROM 1234567890abcdefGHIKL@localhost; + +--error ER_WRONG_STRING_LENGTH +REVOKE EXECUTE ON PROCEDURE t1 FROM some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index f48a210c7eb..bf255bf631a 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -6286,6 +6286,32 @@ select * from (select 1 as a) as t1 natural join (select * from test.t3) as t2| use test| drop table t3| + +# +# Test for BUG#16899: Possible buffer overflow in handling of DEFINER-clause. +# + +# Prepare. + +--disable_warnings +DROP PROCEDURE IF EXISTS bug16899_p1| +DROP FUNCTION IF EXISTS bug16899_f1| +--enable_warnings + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=1234567890abcdefGHIKL@localhost PROCEDURE bug16899_p1() +BEGIN + SET @a = 1; +END| + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY + FUNCTION bug16899_f1() RETURNS INT +BEGIN + RETURN 1; +END| + + # # BUG#NNNN: New bug synopsis # diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test index 95e8eaae83e..7e20b61c1e4 100644 --- a/mysql-test/t/trigger.test +++ b/mysql-test/t/trigger.test @@ -1301,6 +1301,36 @@ create trigger wont_work after update on event for each row begin set @a:= 1; end| +use test| delimiter ;| + +# +# Test for BUG#16899: Possible buffer overflow in handling of DEFINER-clause. +# + +# Prepare. + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +--enable_warnings + +CREATE TABLE t1(c INT); +CREATE TABLE t2(c INT); + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=1234567890abcdefGHIKL@localhost + TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a = 1; + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY + TRIGGER t2_bi BEFORE INSERT ON t2 FOR EACH ROW SET @a = 2; + +# Cleanup. + +DROP TABLE t1; +DROP TABLE t2; + + --echo End of 5.0 tests diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index 5cb85ca6c9b..3e0129dcb7d 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -2718,3 +2718,30 @@ DROP TABLE t1; --disable_warnings DROP VIEW IF EXISTS v1; --enable_warnings + + +# +# Test for BUG#16899: Possible buffer overflow in handling of DEFINER-clause. +# + +# Prepare. + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP VIEW IF EXISTS v1; +DROP VIEW IF EXISTS v2; +--enable_warnings + +CREATE TABLE t1(a INT, b INT); + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=1234567890abcdefGHIKL@localhost + VIEW v1 AS SELECT a FROM t1; + +--error ER_WRONG_STRING_LENGTH +CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY + VIEW v2 AS SELECT b FROM t1; + +# Cleanup. + +DROP TABLE t1; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 0834af46d38..3ec9dd718e8 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -557,6 +557,7 @@ void get_default_definer(THD *thd, LEX_USER *definer); LEX_USER *create_default_definer(THD *thd); LEX_USER *create_definer(THD *thd, LEX_STRING *user_name, LEX_STRING *host_name); LEX_USER *get_current_user(THD *thd, LEX_USER *user); +bool check_string_length(LEX_STRING *str, const char *err_msg, uint max_length); enum enum_mysql_completiontype { ROLLBACK_RELEASE=-2, ROLLBACK=1, ROLLBACK_AND_CHAIN=7, diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index 5c967ba19bd..b3cb33090ce 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -5623,3 +5623,9 @@ ER_NO_TRIGGERS_ON_SYSTEM_SCHEMA eng "Triggers can not be created on system tables" ER_REMOVED_SPACES eng "Leading spaces are removed from name '%s'" +ER_USERNAME + eng "user name" +ER_HOSTNAME + eng "host name" +ER_WRONG_STRING_LENGTH + eng "String '%-.70s' is too long for %s (should be no longer than %d)" diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index ae5ea210a47..0c7b8626c93 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2900,14 +2900,6 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list, result= TRUE; continue; } - if (Str->host.length > HOSTNAME_LENGTH || - Str->user.length > USERNAME_LENGTH) - { - my_message(ER_GRANT_WRONG_HOST_OR_USER, ER(ER_GRANT_WRONG_HOST_OR_USER), - MYF(0)); - result= TRUE; - continue; - } /* Create user if needed */ error=replace_user_table(thd, tables[0].table, *Str, 0, revoke_grant, create_new_users, @@ -3112,15 +3104,6 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, result= TRUE; continue; } - if (Str->host.length > HOSTNAME_LENGTH || - Str->user.length > USERNAME_LENGTH) - { - if (!no_error) - my_message(ER_GRANT_WRONG_HOST_OR_USER, ER(ER_GRANT_WRONG_HOST_OR_USER), - MYF(0)); - result= TRUE; - continue; - } /* Create user if needed */ error=replace_user_table(thd, tables[0].table, *Str, 0, revoke_grant, create_new_users, @@ -3246,14 +3229,6 @@ bool mysql_grant(THD *thd, const char *db, List &list, result= TRUE; continue; } - if (Str->host.length > HOSTNAME_LENGTH || - Str->user.length > USERNAME_LENGTH) - { - my_message(ER_GRANT_WRONG_HOST_OR_USER, ER(ER_GRANT_WRONG_HOST_OR_USER), - MYF(0)); - result= -1; - continue; - } if (replace_user_table(thd, tables[0].table, *Str, (!db ? rights : 0), revoke_grant, create_new_users, test(thd->variables.sql_mode & @@ -4144,14 +4119,6 @@ bool mysql_show_grants(THD *thd,LEX_USER *lex_user) DBUG_RETURN(TRUE); } - if (lex_user->host.length > HOSTNAME_LENGTH || - lex_user->user.length > USERNAME_LENGTH) - { - my_message(ER_GRANT_WRONG_HOST_OR_USER, ER(ER_GRANT_WRONG_HOST_OR_USER), - MYF(0)); - DBUG_RETURN(TRUE); - } - rw_rdlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 061c29e16c3..f3ac1280983 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7533,16 +7533,34 @@ LEX_USER *create_definer(THD *thd, LEX_STRING *user_name, LEX_STRING *host_name) LEX_USER *get_current_user(THD *thd, LEX_USER *user) { - LEX_USER *curr_user; if (!user->user.str) // current_user - { - if (!(curr_user= (LEX_USER*) thd->alloc(sizeof(LEX_USER)))) - { - my_error(ER_OUTOFMEMORY, MYF(0), sizeof(LEX_USER)); - return 0; - } - get_default_definer(thd, curr_user); - return curr_user; - } + return create_default_definer(thd); + return user; } + + +/* + Check that length of a string does not exceed some limit. + + SYNOPSIS + check_string_length() + str string to be checked + err_msg error message to be displayed if the string is too long + max_length max length + + RETURN + FALSE the passed string is not longer than max_length + TRUE the passed string is longer than max_length +*/ + +bool check_string_length(LEX_STRING *str, const char *err_msg, + uint max_length) +{ + if (str->length <= max_length) + return FALSE; + + my_error(ER_WRONG_STRING_LENGTH, MYF(0), str->str, err_msg, max_length); + + return TRUE; +} diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 1dbed6d3cdb..133b6e18fee 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -7511,6 +7511,9 @@ user: $$->user = $1; $$->host.str= (char *) "%"; $$->host.length= 1; + + if (check_string_length(&$$->user, ER(ER_USERNAME), USERNAME_LENGTH)) + YYABORT; } | ident_or_text '@' ident_or_text { @@ -7518,6 +7521,11 @@ user: if (!($$=(LEX_USER*) thd->alloc(sizeof(st_lex_user)))) YYABORT; $$->user = $1; $$->host=$3; + + if (check_string_length(&$$->user, ER(ER_USERNAME), USERNAME_LENGTH) || + check_string_length(&$$->host, ER(ER_HOSTNAME), + HOSTNAME_LENGTH)) + YYABORT; } | CURRENT_USER optional_braces { @@ -8995,15 +9003,9 @@ definer: */ YYTHD->lex->definer= 0; } - | DEFINER_SYM EQ CURRENT_USER optional_braces + | DEFINER_SYM EQ user { - if (! (YYTHD->lex->definer= create_default_definer(YYTHD))) - YYABORT; - } - | DEFINER_SYM EQ ident_or_text '@' ident_or_text - { - if (!(YYTHD->lex->definer= create_definer(YYTHD, &$3, &$5))) - YYABORT; + YYTHD->lex->definer= get_current_user(YYTHD, $3); } ;