Commit 8f395ebb authored by unknown's avatar unknown

Fix for the following bugs:

  - BUG#15166: Wrong update permissions required to execute triggers
  - BUG#15196: Wrong select permission required to execute triggers

The idea of the fix is to check necessary privileges
in Item_trigger_field::fix_fields(), instead of having "special variables"
technique. To achieve this, we should pass to an Item_trigger_field instance
a flag, which will indicate the usage/access type of this trigger variable.


mysql-test/r/trigger-grant.result:
  Update the result file.
mysql-test/t/trigger-grant.test:
  Add test cases for BUG#15166 and BUG#15196
sql/item.cc:
  Item_trigger_field: check appropriate (SELECT/UPDATE) privilege in fix_fields().
sql/item.h:
  Add a flag to specify access type for trigger field.
sql/sql_trigger.cc:
  "Special variable" technique of checking privileges for NEW/OLD variables
  was replaced by checking table- and column-level privileges in
  Item_trigger_field::fix_fields().
sql/sql_trigger.h:
  "Special variable" technique of checking privileges for NEW/OLD variables
  was replaced by checking table- and column-level privileges in
  Item_trigger_field::fix_fields().
sql/sql_yacc.yy:
  Specify access type for trigger fields.
parent 9e0240d3
This diff is collapsed.
This diff is collapsed.
...@@ -5215,6 +5215,7 @@ void Item_insert_value::print(String *str) ...@@ -5215,6 +5215,7 @@ void Item_insert_value::print(String *str)
setup_field() setup_field()
thd - current thread context thd - current thread context
table - table of trigger (and where we looking for fields) table - table of trigger (and where we looking for fields)
table_grant_info - GRANT_INFO of the subject table
NOTE NOTE
This function does almost the same as fix_fields() for Item_field This function does almost the same as fix_fields() for Item_field
...@@ -5228,7 +5229,8 @@ void Item_insert_value::print(String *str) ...@@ -5228,7 +5229,8 @@ void Item_insert_value::print(String *str)
table of trigger which uses this item. table of trigger which uses this item.
*/ */
void Item_trigger_field::setup_field(THD *thd, TABLE *table) void Item_trigger_field::setup_field(THD *thd, TABLE *table,
GRANT_INFO *table_grant_info)
{ {
bool save_set_query_id= thd->set_query_id; bool save_set_query_id= thd->set_query_id;
...@@ -5242,6 +5244,7 @@ void Item_trigger_field::setup_field(THD *thd, TABLE *table) ...@@ -5242,6 +5244,7 @@ void Item_trigger_field::setup_field(THD *thd, TABLE *table)
0, &field_idx); 0, &field_idx);
thd->set_query_id= save_set_query_id; thd->set_query_id= save_set_query_id;
triggers= table->triggers; triggers= table->triggers;
table_grants= table_grant_info;
} }
...@@ -5260,22 +5263,42 @@ bool Item_trigger_field::fix_fields(THD *thd, Item **items) ...@@ -5260,22 +5263,42 @@ bool Item_trigger_field::fix_fields(THD *thd, Item **items)
Since trigger is object tightly associated with TABLE object most Since trigger is object tightly associated with TABLE object most
of its set up can be performed during trigger loading i.e. trigger of its set up can be performed during trigger loading i.e. trigger
parsing! So we have little to do in fix_fields. :) parsing! So we have little to do in fix_fields. :)
FIXME may be we still should bother about permissions here.
*/ */
DBUG_ASSERT(fixed == 0); DBUG_ASSERT(fixed == 0);
/* Set field. */
if (field_idx != (uint)-1) if (field_idx != (uint)-1)
{ {
#ifndef NO_EMBEDDED_ACCESS_CHECKS
/*
Check access privileges for the subject table. We check privileges only
in runtime.
*/
if (table_grants)
{
table_grants->want_privilege=
access_type == AT_READ ? SELECT_ACL : UPDATE_ACL;
if (check_grant_column(thd, table_grants, triggers->table->s->db,
triggers->table->s->table_name, field_name,
strlen(field_name), thd->security_ctx))
return TRUE;
}
#endif // NO_EMBEDDED_ACCESS_CHECKS
field= (row_version == OLD_ROW) ? triggers->old_field[field_idx] : field= (row_version == OLD_ROW) ? triggers->old_field[field_idx] :
triggers->new_field[field_idx]; triggers->new_field[field_idx];
set_field(field); set_field(field);
fixed= 1; fixed= 1;
return 0; return FALSE;
} }
my_error(ER_BAD_FIELD_ERROR, MYF(0), field_name, my_error(ER_BAD_FIELD_ERROR, MYF(0), field_name,
(row_version == NEW_ROW) ? "NEW" : "OLD"); (row_version == NEW_ROW) ? "NEW" : "OLD");
return 1; return TRUE;
} }
......
...@@ -2126,6 +2126,8 @@ class Item_trigger_field : public Item_field ...@@ -2126,6 +2126,8 @@ class Item_trigger_field : public Item_field
/* Is this item represents row from NEW or OLD row ? */ /* Is this item represents row from NEW or OLD row ? */
enum row_version_type {OLD_ROW, NEW_ROW}; enum row_version_type {OLD_ROW, NEW_ROW};
row_version_type row_version; row_version_type row_version;
/* Is this item used for reading or updating the value? */
enum access_types { AT_READ = 0x1, AT_UPDATE = 0x2 };
/* Next in list of all Item_trigger_field's in trigger */ /* Next in list of all Item_trigger_field's in trigger */
Item_trigger_field *next_trg_field; Item_trigger_field *next_trg_field;
/* Index of the field in the TABLE::field array */ /* Index of the field in the TABLE::field array */
...@@ -2135,18 +2137,24 @@ class Item_trigger_field : public Item_field ...@@ -2135,18 +2137,24 @@ class Item_trigger_field : public Item_field
Item_trigger_field(Name_resolution_context *context_arg, Item_trigger_field(Name_resolution_context *context_arg,
row_version_type row_ver_arg, row_version_type row_ver_arg,
const char *field_name_arg) const char *field_name_arg,
access_types access_type_arg)
:Item_field(context_arg, :Item_field(context_arg,
(const char *)NULL, (const char *)NULL, field_name_arg), (const char *)NULL, (const char *)NULL, field_name_arg),
row_version(row_ver_arg), field_idx((uint)-1) row_version(row_ver_arg), field_idx((uint)-1),
access_type(access_type_arg), table_grants(NULL)
{} {}
void setup_field(THD *thd, TABLE *table); void setup_field(THD *thd, TABLE *table, GRANT_INFO *table_grant_info);
enum Type type() const { return TRIGGER_FIELD_ITEM; } enum Type type() const { return TRIGGER_FIELD_ITEM; }
bool eq(const Item *item, bool binary_cmp) const; bool eq(const Item *item, bool binary_cmp) const;
bool fix_fields(THD *, Item **); bool fix_fields(THD *, Item **);
void print(String *str); void print(String *str);
table_map used_tables() const { return (table_map)0L; } table_map used_tables() const { return (table_map)0L; }
void cleanup(); void cleanup();
private:
access_types access_type;
GRANT_INFO *table_grants;
}; };
......
...@@ -381,7 +381,12 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables, ...@@ -381,7 +381,12 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables,
for (trg_field= (Item_trigger_field *)(lex->trg_table_fields.first); for (trg_field= (Item_trigger_field *)(lex->trg_table_fields.first);
trg_field; trg_field= trg_field->next_trg_field) trg_field; trg_field= trg_field->next_trg_field)
{ {
trg_field->setup_field(thd, table); /*
NOTE: now we do not check privileges at CREATE TRIGGER time. This will
be changed in the future.
*/
trg_field->setup_field(thd, table, NULL);
if (!trg_field->fixed && if (!trg_field->fixed &&
trg_field->fix_fields(thd, (Item **)0)) trg_field->fix_fields(thd, (Item **)0))
return 1; return 1;
...@@ -828,8 +833,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, ...@@ -828,8 +833,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,
char *trg_name_buff; char *trg_name_buff;
List_iterator_fast<ulonglong> itm(triggers->definition_modes_list); List_iterator_fast<ulonglong> itm(triggers->definition_modes_list);
List_iterator_fast<LEX_STRING> it_definer(triggers-> List_iterator_fast<LEX_STRING> it_definer(triggers->definers_list);
definers_list);
LEX *old_lex= thd->lex, lex; LEX *old_lex= thd->lex, lex;
sp_rcontext *save_spcont= thd->spcont; sp_rcontext *save_spcont= thd->spcont;
ulong save_sql_mode= thd->variables.sql_mode; ulong save_sql_mode= thd->variables.sql_mode;
...@@ -844,6 +848,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, ...@@ -844,6 +848,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,
{ {
trg_sql_mode= itm++; trg_sql_mode= itm++;
LEX_STRING *trg_definer= it_definer++; LEX_STRING *trg_definer= it_definer++;
thd->variables.sql_mode= (ulong)*trg_sql_mode; thd->variables.sql_mode= (ulong)*trg_sql_mode;
lex_start(thd, (uchar*)trg_create_str->str, trg_create_str->length); lex_start(thd, (uchar*)trg_create_str->str, trg_create_str->length);
...@@ -917,11 +922,11 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, ...@@ -917,11 +922,11 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,
(Item_trigger_field *)(lex.trg_table_fields.first); (Item_trigger_field *)(lex.trg_table_fields.first);
trg_field; trg_field;
trg_field= trg_field->next_trg_field) trg_field= trg_field->next_trg_field)
trg_field->setup_field(thd, table); {
trg_field->setup_field(thd, table,
triggers->m_spec_var_used[lex.trg_chistics.event] &triggers->subject_table_grants[lex.trg_chistics.event]
[lex.trg_chistics.action_time]= [lex.trg_chistics.action_time]);
lex.trg_table_fields.first ? TRUE : FALSE; }
lex_end(&lex); lex_end(&lex);
} }
...@@ -1172,33 +1177,14 @@ bool Table_triggers_list::process_triggers(THD *thd, trg_event_type event, ...@@ -1172,33 +1177,14 @@ bool Table_triggers_list::process_triggers(THD *thd, trg_event_type event,
} }
/* /*
If the trigger uses special variables (NEW/OLD), check that we have Fetch information about table-level privileges to GRANT_INFO structure for
SELECT and UPDATE privileges on the subject table. subject table. Check of privileges that will use it and information about
column-level privileges will happen in Item_trigger_field::fix_fields().
*/ */
if (is_special_var_used(event, time_type)) fill_effective_table_privileges(thd,
{ &subject_table_grants[event][time_type],
TABLE_LIST table_list, **save_query_tables_own_last; table->s->db, table->s->table_name);
bzero((char *) &table_list, sizeof (table_list));
table_list.db= (char *) table->s->db;
table_list.db_length= strlen(table_list.db);
table_list.table_name= (char *) table->s->table_name;
table_list.table_name_length= strlen(table_list.table_name);
table_list.alias= (char *) table->alias;
table_list.table= table;
save_query_tables_own_last= thd->lex->query_tables_own_last;
thd->lex->query_tables_own_last= 0;
err_status= check_table_access(thd, SELECT_ACL | UPDATE_ACL,
&table_list, 0);
thd->lex->query_tables_own_last= save_query_tables_own_last;
if (err_status)
{
sp_restore_security_context(thd, save_ctx);
return TRUE;
}
}
#endif // NO_EMBEDDED_ACCESS_CHECKS #endif // NO_EMBEDDED_ACCESS_CHECKS
thd->reset_sub_statement_state(&statement_state, SUB_STMT_TRIGGER); thd->reset_sub_statement_state(&statement_state, SUB_STMT_TRIGGER);
......
...@@ -56,10 +56,9 @@ class Table_triggers_list: public Sql_alloc ...@@ -56,10 +56,9 @@ class Table_triggers_list: public Sql_alloc
LEX_STRING sroutines_key; LEX_STRING sroutines_key;
/* /*
is_special_var_used specifies whether trigger body contains special Grant information for each trigger (pair: subject table, trigger definer).
variables (NEW/OLD).
*/ */
bool m_spec_var_used[TRG_EVENT_MAX][TRG_ACTION_MAX]; GRANT_INFO subject_table_grants[TRG_EVENT_MAX][TRG_ACTION_MAX];
public: public:
/* /*
...@@ -78,6 +77,7 @@ class Table_triggers_list: public Sql_alloc ...@@ -78,6 +77,7 @@ class Table_triggers_list: public Sql_alloc
record1_field(0), table(table_arg) record1_field(0), table(table_arg)
{ {
bzero((char *)bodies, sizeof(bodies)); bzero((char *)bodies, sizeof(bodies));
bzero((char *)&subject_table_grants, sizeof(subject_table_grants));
} }
~Table_triggers_list(); ~Table_triggers_list();
...@@ -109,11 +109,6 @@ class Table_triggers_list: public Sql_alloc ...@@ -109,11 +109,6 @@ class Table_triggers_list: public Sql_alloc
return test(bodies[TRG_EVENT_UPDATE][TRG_ACTION_BEFORE]); return test(bodies[TRG_EVENT_UPDATE][TRG_ACTION_BEFORE]);
} }
inline bool is_special_var_used(int event, int action_time) const
{
return m_spec_var_used[event][action_time];
}
void set_table(TABLE *new_table); void set_table(TABLE *new_table);
friend class Item_trigger_field; friend class Item_trigger_field;
......
...@@ -7303,7 +7303,8 @@ simple_ident_q: ...@@ -7303,7 +7303,8 @@ simple_ident_q:
new_row ? new_row ?
Item_trigger_field::NEW_ROW: Item_trigger_field::NEW_ROW:
Item_trigger_field::OLD_ROW, Item_trigger_field::OLD_ROW,
$3.str))) $3.str,
Item_trigger_field::AT_READ)))
YYABORT; YYABORT;
/* /*
...@@ -7929,7 +7930,9 @@ sys_option_value: ...@@ -7929,7 +7930,9 @@ sys_option_value:
if (!(trg_fld= new Item_trigger_field(Lex->current_context(), if (!(trg_fld= new Item_trigger_field(Lex->current_context(),
Item_trigger_field::NEW_ROW, Item_trigger_field::NEW_ROW,
$2.base_name.str)) || $2.base_name.str,
Item_trigger_field::AT_UPDATE)
) ||
!(sp_fld= new sp_instr_set_trigger_field(lex->sphead-> !(sp_fld= new sp_instr_set_trigger_field(lex->sphead->
instructions(), instructions(),
lex->spcont, lex->spcont,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment