Skip to content

Commit

Permalink
Bug#35507223 MySQL Server crashes when executing query
Browse files Browse the repository at this point in the history
ISSUE
=====
The expression ~NO_ACCESS was being used to provide an "all access"
ACL bitmask.  This usage was successful when an ACL bitmask was a 32
bit value, but can cause problems now an ACL bitmask is a 64 bit value
(on Linux, not on Windows) as ~NO_ACCESS produces a value with the
high order 32 bits set.

SOLUTION
========
Make the type of all ACL bitmask variables explicitly 32 bits, by
changing the relevant bitmask variable types from ulong to uint32_t.

Change-Id: I7abe9ac400490de1d855a23ff5ced1c9124f1453
  • Loading branch information
Daniel Blanchard committed Apr 29, 2024
1 parent 75acf5e commit 3a27636
Show file tree
Hide file tree
Showing 47 changed files with 697 additions and 576 deletions.
4 changes: 4 additions & 0 deletions mysql-test/r/bug35507223.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
CREATE TABLE v0(c1 INT);
CREATE TABLE IF NOT EXISTS v2 ( CHECK ( c1 IN ( SELECT DISTINCT * FROM ( v0 a3 ) CROSS JOIN ( v0 ) ON c1 WHERE c1 ) ) ) TABLE v0 ;
ERROR 23000: Column 'c1' in field list is ambiguous
DROP TABLE v0;
6 changes: 6 additions & 0 deletions mysql-test/t/bug35507223.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CREATE TABLE v0(c1 INT);
# Expect an error
# ERROR: 1052 (23000): Column 'c1' in field list is ambiguous
--error ER_NON_UNIQ_ERROR
CREATE TABLE IF NOT EXISTS v2 ( CHECK ( c1 IN ( SELECT DISTINCT * FROM ( v0 a3 ) CROSS JOIN ( v0 ) ON c1 WHERE c1 ) ) ) TABLE v0 ;
DROP TABLE v0;
31 changes: 17 additions & 14 deletions sql/auth/acl_table_user.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ static std::map<const User_attribute_type, const std::string>

Acl_user_attributes::Acl_user_attributes(MEM_ROOT *mem_root,
bool read_restrictions,
Auth_id &auth_id, ulong global_privs)
Auth_id &auth_id,
Access_bitmask global_privs)
: m_mem_root(mem_root),
m_read_restrictions(read_restrictions),
m_auth_id(auth_id),
Expand All @@ -149,7 +150,7 @@ Acl_user_attributes::Acl_user_attributes(MEM_ROOT *mem_root,
Auth_id &auth_id,
Restrictions *restrictions,
I_multi_factor_auth *mfa)
: Acl_user_attributes(mem_root, read_restrictions, auth_id, ~NO_ACCESS) {
: Acl_user_attributes(mem_root, read_restrictions, auth_id, ALL_ACCESS) {
if (restrictions) m_restrictions = *restrictions;
m_mfa = mfa;
}
Expand All @@ -176,16 +177,16 @@ bool Acl_user_attributes::consume_user_attributes_json(Json_dom_ptr json) {
}

void Acl_user_attributes::report_and_remove_invalid_db_restrictions(
DB_restrictions &db_restrictions, ulong mask, enum loglevel level,
DB_restrictions &db_restrictions, Access_bitmask mask, enum loglevel level,
ulonglong errcode) {
if (!db_restrictions.is_empty()) {
for (auto &itr : db_restrictions()) {
ulong privs = itr.second;
Access_bitmask privs = itr.second;
if (privs != (privs & mask)) {
std::string invalid_privs;
std::string separator(", ");
bool second = false;
ulong filtered_privs = privs & ~mask;
Access_bitmask filtered_privs = privs & ~mask;
if (filtered_privs)
db_restrictions.remove(itr.first.c_str(), filtered_privs);
while (filtered_privs != 0) {
Expand Down Expand Up @@ -461,8 +462,9 @@ Acl_table_user_writer_status::Acl_table_user_writer_status()
methods
*/
Acl_table_user_writer::Acl_table_user_writer(
THD *thd, TABLE *table, LEX_USER *combo, ulong rights, bool revoke_grant,
bool can_create_user, Pod_user_what_to_update what_to_update,
THD *thd, TABLE *table, LEX_USER *combo, Access_bitmask rights,
bool revoke_grant, bool can_create_user,
Pod_user_what_to_update what_to_update,
Restrictions *restrictions = nullptr, I_multi_factor_auth *mfa = nullptr)
: Acl_table(thd, table, acl_table::Acl_table_operation::OP_INSERT),
m_has_user_application_user_metadata(false),
Expand Down Expand Up @@ -858,7 +860,7 @@ bool Acl_table_user_writer::update_privileges(
/* Update table columns with new privileges */
char what = m_revoke_grant ? 'N' : 'Y';
Field **tmp_field;
ulong priv;
Access_bitmask priv;
for (tmp_field = m_table->field + 2, priv = SELECT_ACL;
*tmp_field && (*tmp_field)->real_type() == MYSQL_TYPE_ENUM &&
((Field_enum *)(*tmp_field))->typelib->count == 2;
Expand All @@ -885,8 +887,8 @@ bool Acl_table_user_writer::update_privileges(
}

return_value.updated_rights = get_user_privileges();
DBUG_PRINT("info",
("Privileges on disk are now %lu", return_value.updated_rights));
DBUG_PRINT("info", ("Privileges on disk are now %" PRIu32,
return_value.updated_rights));
DBUG_PRINT("info", ("table fields: %d", m_table->s->fields));

return false;
Expand Down Expand Up @@ -1300,10 +1302,10 @@ bool Acl_table_user_writer::write_user_attributes_column(
@returns Bitmask representing global privileges granted to given account
*/
ulong Acl_table_user_writer::get_user_privileges() {
Access_bitmask Acl_table_user_writer::get_user_privileges() {
uint next_field;
char *priv_str;
ulong rights =
Access_bitmask rights =
get_access(m_table, m_table_schema->select_priv_idx(), &next_field);
if (m_table->s->fields > m_table_schema->drop_role_priv_idx()) {
priv_str =
Expand Down Expand Up @@ -2124,8 +2126,9 @@ Password_lock::Password_lock(Password_lock &&other) {
processing subsequent user specified in the ACL statement.
*/

int replace_user_table(THD *thd, TABLE *table, LEX_USER *combo, ulong rights,
bool revoke_grant, bool can_create_user,
int replace_user_table(THD *thd, TABLE *table, LEX_USER *combo,
Access_bitmask rights, bool revoke_grant,
bool can_create_user,
acl_table::Pod_user_what_to_update &what_to_update,
Restrictions *restrictions /*= nullptr*/,
I_multi_factor_auth *mfa /*= nullptr*/) {
Expand Down
22 changes: 12 additions & 10 deletions sql/auth/acl_table_user.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Acl_user_attributes {
Default constructor.
*/
Acl_user_attributes(MEM_ROOT *mem_root, bool read_restrictions,
Auth_id &auth_id, ulong global_privs);
Auth_id &auth_id, Access_bitmask global_privs);

Acl_user_attributes(MEM_ROOT *mem_root, bool read_restrictions,
Auth_id &auth_id, Restrictions *m_restrictions,
Expand Down Expand Up @@ -178,8 +178,8 @@ class Acl_user_attributes {

private:
void report_and_remove_invalid_db_restrictions(
DB_restrictions &db_restrictions, ulong mask, enum loglevel level,
ulonglong errcode);
DB_restrictions &db_restrictions, Access_bitmask mask,
enum loglevel level, ulonglong errcode);
bool deserialize_password_lock(const Json_object &json_object);
bool deserialize_multi_factor(const Json_object &json_object);

Expand All @@ -195,7 +195,7 @@ class Acl_user_attributes {
/** Restrictions_list on certain databases for user */
Restrictions m_restrictions;
/** Global static privileges */
ulong m_global_privs;
Access_bitmask m_global_privs;
/** password locking */
Password_lock m_password_lock;
/** multi factor auth info */
Expand All @@ -215,7 +215,8 @@ using acl_table_user_writer_status =
class Acl_table_user_writer_status {
public:
Acl_table_user_writer_status();
Acl_table_user_writer_status(bool skip, ulong rights, Table_op_error_code err,
Acl_table_user_writer_status(bool skip, Access_bitmask rights,
Table_op_error_code err,
my_timeval pwd_timestamp, std::string cred,
Password_lock &password_lock,
I_multi_factor_auth *multi_factor)
Expand All @@ -229,7 +230,7 @@ class Acl_table_user_writer_status {
multi_factor(multi_factor) {}

bool skip_cache_update;
ulong updated_rights;
Access_bitmask updated_rights;
Table_op_error_code error;
my_timeval password_change_timestamp;
std::string second_cred;
Expand All @@ -240,8 +241,9 @@ class Acl_table_user_writer_status {

class Acl_table_user_writer : public Acl_table {
public:
Acl_table_user_writer(THD *thd, TABLE *table, LEX_USER *combo, ulong rights,
bool revoke_grant, bool can_create_user,
Acl_table_user_writer(THD *thd, TABLE *table, LEX_USER *combo,
Access_bitmask rights, bool revoke_grant,
bool can_create_user,
Pod_user_what_to_update what_to_update,
Restrictions *restrictions, I_multi_factor_auth *mfa);
~Acl_table_user_writer() override;
Expand All @@ -265,15 +267,15 @@ class Acl_table_user_writer : public Acl_table {

void replace_user_application_user_metadata(
std::function<bool(TABLE *table)> const &update);
ulong get_user_privileges();
Access_bitmask get_user_privileges();
std::string get_current_credentials();

private:
bool update_user_application_user_metadata();
bool write_user_attributes_column(const Acl_user_attributes &user_attributes);
bool m_has_user_application_user_metadata;
LEX_USER *m_combo;
ulong m_rights;
Access_bitmask m_rights;
bool m_revoke_grant;
bool m_can_create_user;
Pod_user_what_to_update m_what_to_update;
Expand Down
68 changes: 36 additions & 32 deletions sql/auth/auth_acls.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,26 @@
#ifndef AUTH_ACLS_INCLUDED
#define AUTH_ACLS_INCLUDED

#include <cstdint>
#include <string>
#include <unordered_map>
#include <vector>

/* Total Number of ACLs present in mysql.user */
#define NUM_ACLS 31

#define SELECT_ACL (1L << 0)
#define INSERT_ACL (1L << 1)
#define UPDATE_ACL (1L << 2)
#define DELETE_ACL (1L << 3)
#define CREATE_ACL (1L << 4)
#define DROP_ACL (1L << 5)
#define RELOAD_ACL (1L << 6)
#define SHUTDOWN_ACL (1L << 7)
#define PROCESS_ACL (1L << 8)
#define FILE_ACL (1L << 9)
typedef uint32_t Access_bitmask;

#define SELECT_ACL ((Access_bitmask)1 << 0)
#define INSERT_ACL ((Access_bitmask)1 << 1)
#define UPDATE_ACL ((Access_bitmask)1 << 2)
#define DELETE_ACL ((Access_bitmask)1 << 3)
#define CREATE_ACL ((Access_bitmask)1 << 4)
#define DROP_ACL ((Access_bitmask)1 << 5)
#define RELOAD_ACL ((Access_bitmask)1 << 6)
#define SHUTDOWN_ACL ((Access_bitmask)1 << 7)
#define PROCESS_ACL ((Access_bitmask)1 << 8)
#define FILE_ACL ((Access_bitmask)1 << 9)
/** Set to true by both
GRANT GRANT OPTION ... TO ...
and
Expand All @@ -58,27 +61,27 @@ and
@sa @ref LEX::grant_privilege
*/
#define GRANT_ACL (1L << 10)
#define REFERENCES_ACL (1L << 11)
#define INDEX_ACL (1L << 12)
#define ALTER_ACL (1L << 13)
#define SHOW_DB_ACL (1L << 14)
#define SUPER_ACL (1L << 15)
#define CREATE_TMP_ACL (1L << 16)
#define LOCK_TABLES_ACL (1L << 17)
#define EXECUTE_ACL (1L << 18)
#define REPL_SLAVE_ACL (1L << 19)
#define REPL_CLIENT_ACL (1L << 20)
#define CREATE_VIEW_ACL (1L << 21)
#define SHOW_VIEW_ACL (1L << 22)
#define CREATE_PROC_ACL (1L << 23)
#define ALTER_PROC_ACL (1L << 24)
#define CREATE_USER_ACL (1L << 25)
#define EVENT_ACL (1L << 26)
#define TRIGGER_ACL (1L << 27)
#define CREATE_TABLESPACE_ACL (1L << 28)
#define CREATE_ROLE_ACL (1L << 29)
#define DROP_ROLE_ACL (1L << 30)
#define GRANT_ACL ((Access_bitmask)1 << 10)
#define REFERENCES_ACL ((Access_bitmask)1 << 11)
#define INDEX_ACL ((Access_bitmask)1 << 12)
#define ALTER_ACL ((Access_bitmask)1 << 13)
#define SHOW_DB_ACL ((Access_bitmask)1 << 14)
#define SUPER_ACL ((Access_bitmask)1 << 15)
#define CREATE_TMP_ACL ((Access_bitmask)1 << 16)
#define LOCK_TABLES_ACL ((Access_bitmask)1 << 17)
#define EXECUTE_ACL ((Access_bitmask)1 << 18)
#define REPL_SLAVE_ACL ((Access_bitmask)1 << 19)
#define REPL_CLIENT_ACL ((Access_bitmask)1 << 20)
#define CREATE_VIEW_ACL ((Access_bitmask)1 << 21)
#define SHOW_VIEW_ACL ((Access_bitmask)1 << 22)
#define CREATE_PROC_ACL ((Access_bitmask)1 << 23)
#define ALTER_PROC_ACL ((Access_bitmask)1 << 24)
#define CREATE_USER_ACL ((Access_bitmask)1 << 25)
#define EVENT_ACL ((Access_bitmask)1 << 26)
#define TRIGGER_ACL ((Access_bitmask)1 << 27)
#define CREATE_TABLESPACE_ACL ((Access_bitmask)1 << 28)
#define CREATE_ROLE_ACL ((Access_bitmask)1 << 29)
#define DROP_ROLE_ACL ((Access_bitmask)1 << 30)
/*
don't forget to update
1. static struct show_privileges_st sys_privileges[]
Expand All @@ -89,7 +92,8 @@ and
6. global_privileges map and vector
*/

#define NO_ACCESS (1L << 31)
#define NO_ACCESS ((Access_bitmask)1 << NUM_ACLS)
#define ALL_ACCESS (NO_ACCESS - 1)

/**
Privileges to perform database related operations.
Expand Down
4 changes: 2 additions & 2 deletions sql/auth/auth_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ const std::string &Auth_id::host() const { return m_host; }
@returns Name for the privilege represented by LSB
*/
std::string get_one_priv(ulong &revoke_privs) {
std::string get_one_priv(Access_bitmask &revoke_privs) {
std::string priv;
ulong lsb;
Access_bitmask lsb;
if (revoke_privs != 0) {
// find out the least significant bit(lsb)
lsb = revoke_privs & ~(revoke_privs - 1);
Expand Down
Loading

0 comments on commit 3a27636

Please sign in to comment.