Skip to content

Commit

Permalink
Merge pull request #163 from FirebirdSQL/trigger1_refac
Browse files Browse the repository at this point in the history 
Fixed CORE-5852: There is no check of existance generator and exception when privileges are granted
Moved check of the object existance from trigger1 to grantRevoke method.
  • Loading branch information
@romansimakov
romansimakov committed Jun 22, 2018
2 parents 83c227d + 732c39b commit 364e29a
Show file tree
Hide file tree
Showing 5 changed files with 667 additions and 830 deletions.
104 changes: 97 additions & 7 deletions src/dsql/DdlNodes.epp
View file
Expand Up @@ -11345,11 +11345,51 @@ static bool checkObjectExist(thread_db* tdbb, jrd_tra* transaction, const MetaNa
END_FOR
break;
}
case obj_exception:
{
AutoCacheRequest request(tdbb, drq_exception_exist, DYN_REQUESTS);
FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction)
X IN RDB$EXCEPTIONS
WITH X.RDB$EXCEPTION_NAME EQ name.c_str()
{
rc = true;
}
END_FOR
break;
}
case obj_generator:
{
AutoCacheRequest request(tdbb, drq_generator_exist, DYN_REQUESTS);
FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction)
X IN RDB$GENERATORS
WITH X.RDB$GENERATOR_NAME EQ name.c_str()
{
rc = true;
}
END_FOR
break;
}
}

return rc;
}

static bool checkFieldExist(thread_db* tdbb, jrd_tra* transaction, const MetaName& relation, const MetaName& field)
{
bool rc = false;

AutoCacheRequest request(tdbb, drq_rel_field_exist, DYN_REQUESTS);
FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction)
X IN RDB$RELATION_FIELDS
WITH X.RDB$RELATION_NAME EQ relation.c_str() AND
X.RDB$FIELD_NAME EQ field.c_str()
{
rc = true;
}
END_FOR

return rc;
}

// Execute SQL grant/revoke operation.
void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const GranteeClause* object,
Expand All @@ -11360,6 +11400,7 @@ void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const G
MetaName user(userNod->second);
MetaName dummyName;
const SSHORT objType = object ? object->first : obj_type_MAX;
const MetaName objName(object->second);

This comment has been minimized.

Sign in to view

Copy link
@hvlad

hvlad Jun 24, 2018

Member

It crashes engine when object == NULL.
To reproduce, run "revoke all on all from u01".
U01 not necessary to be existent user name.
bool crdb = false;

char privileges[16];
Expand Down Expand Up @@ -11388,6 +11429,7 @@ void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const G
memmove(cPtr, cPtr + 1, len);
}

// Check if grant object exists
switch (userType)
{
case obj_user_or_role:
Expand All @@ -11411,27 +11453,27 @@ void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const G

case obj_udf:
if (!checkObjectExist(tdbb, transaction, user, userType))
status_exception::raise(Arg::PrivateDyn(301) << user.c_str());
status_exception::raise(Arg::PrivateDyn(301) << user.c_str()); // Function @1 does not exist
break;

case obj_procedure:
if (!checkObjectExist(tdbb, transaction, user, userType))
status_exception::raise(Arg::PrivateDyn(302) << user.c_str());
status_exception::raise(Arg::PrivateDyn(302) << user.c_str()); // Procedure @1 does not exist
break;

case obj_package_header:
if (!checkObjectExist(tdbb, transaction, user, userType))
status_exception::raise(Arg::PrivateDyn(303) << user.c_str());
status_exception::raise(Arg::PrivateDyn(303) << user.c_str()); // Package @1 does not exist
break;

case obj_trigger:
if (!checkObjectExist(tdbb, transaction, user, userType))
status_exception::raise(Arg::PrivateDyn(304) << user.c_str());
status_exception::raise(Arg::PrivateDyn(304) << user.c_str()); // Trigger @1 does not exist
break;

case obj_view:
if (!checkObjectExist(tdbb, transaction, user, userType))
status_exception::raise(Arg::PrivateDyn(305) << user.c_str());
status_exception::raise(Arg::PrivateDyn(305) << user.c_str()); // View @1 does not exist
break;

case obj_sql_role:
Expand All @@ -11456,6 +11498,56 @@ void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const G

}

// Check if grant subject exists
switch (objType)
{
case obj_view:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(305) << objName.c_str()); // View @1 does not exist
break;

case obj_relation:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(306) << objName.c_str()); // Table @1 does not exist

if (field.hasData() && !checkFieldExist(tdbb, transaction, objName, field))
status_exception::raise(Arg::PrivateDyn(309) << field.c_str() << objName.c_str()); // Field @1 of table @2 does not exist
break;

case obj_trigger:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(304) << objName.c_str()); // Trigger @1 does not exist
break;

case obj_procedure:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(302) << objName.c_str()); // Procedure @1 does not exist
break;

case obj_exception:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(307) << objName.c_str()); // Exception @1 does not exist
break;

case obj_generator:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(308) << objName.c_str()); // Generator/Sequence @1 does not exist
break;

case obj_udf:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(301) << objName.c_str()); // Function @1 does not exist
break;

case obj_package_header:
if (!checkObjectExist(tdbb, transaction, objName, objType))
status_exception::raise(Arg::PrivateDyn(303) << objName.c_str()); // Package @1 does not exist
break;

default:
fb_assert(false);
}

if (options == 1) // with grant option
{
switch (userType)
Expand Down Expand Up @@ -11533,8 +11625,6 @@ void GrantRevokeNode::grantRevoke(thread_db* tdbb, jrd_tra* transaction, const G
return;
}

const MetaName objName(object->second);

if (objType == obj_sql_role && objName == NULL_ROLE)
{
if (isGrant)
Expand Down
3 changes: 3 additions & 0 deletions src/jrd/drq.h
View file
Expand Up @@ -240,6 +240,9 @@ enum drq_type_t
drq_package_exist, // check if package exists
drq_trigger_exist, // check if trigger exists
drq_rel_exist, // check if relation or view exists
drq_exception_exist, // check if exception exists
drq_generator_exist, // check if generator exists
drq_rel_field_exist, // check if a field of relation or view exists

drq_MAX
};
Expand Down

0 comments on commit 364e29a

Please sign in to comment.