Fixed CORE-5823: No permission for SELECT access to blob field in stored procedure. And CORE-5841: No permission for SELECT access to TABLE PLG$SRP in newer snapshot. (#170)

src/dsql/ExprNodes.cpp

@@ -8935,7 +8935,7 @@ dsc* ParameterNode::execute(thread_db* tdbb, jrd_req* request) const
 				if (!blobId->isEmpty())
 				{
 					if (!request->hasInternalStatement())
-						tdbb->getTransaction()->checkBlob(tdbb, blobId);
+						tdbb->getTransaction()->checkBlob(tdbb, blobId, NULL, true);
 
 					if (desc->getCharSet() != CS_NONE && desc->getCharSet() != CS_BINARY)
 					{

src/jrd/RecordNumber.h

@@ -307,6 +307,13 @@ struct bid
 		return bid_quad.bid_quad_high == other.bid_quad.bid_quad_high &&
 			bid_quad.bid_quad_low == other.bid_quad.bid_quad_low;
 	}
+
+	bool operator > (const bid& other) const
+	{
+		return bid_quad.bid_quad_high > other.bid_quad.bid_quad_high ||
+				(bid_quad.bid_quad_high == other.bid_quad.bid_quad_high &&
+				bid_quad.bid_quad_low > other.bid_quad.bid_quad_low);
+	}
 };
 
 } // namespace Jrd

src/jrd/blb.cpp

@@ -1403,16 +1403,7 @@ blb* blb::open2(thread_db* tdbb,
 		if (!blob->blb_relation->isSystem() && blob->blb_fld_id < blob->blb_relation->rel_fields->count())
 		{
 			jrd_fld* fld = (*blob->blb_relation->rel_fields)[blob->blb_fld_id];
-			if (fld->fld_security_name.hasData())
+			transaction->checkBlob(tdbb, &blobId, fld, true);
-			{
-				SecurityClass* s_class = SCL_get_class(tdbb, fld->fld_security_name.c_str());
-				if (s_class && !s_class->scl_blb_access)
-				{
-					SCL_check_access(tdbb, s_class, 0, 0, NULL, SCL_select, SCL_object_column, false,
-						fld->fld_name, blob->blb_relation->rel_name);
-					s_class->scl_blb_access = true;
-				}
-			}
 		}
 #endif
 

src/jrd/exe.cpp

@@ -705,6 +705,10 @@ void EXE_receive(thread_db* tdbb,
 						current->bli_request = NULL;
 					}
 				}
+				else
+				{
+					transaction->checkBlob(tdbb, id, NULL, false);
+				}
 			}
 		}
 	}

src/jrd/jrd.cpp

@@ -3315,7 +3315,7 @@ JBlob* JAttachment::openBlob(CheckStatusWrapper* user_status, ITransaction* tra,
 			const bid* id = reinterpret_cast<bid*>(blob_id);
 
 			if (blob_id->gds_quad_high)
-				transaction->checkBlob(tdbb, id);
+				transaction->checkBlob(tdbb, id, NULL, true);
 
 			blob = blb::open2(tdbb, transaction, id, bpb_length, bpb, true);
 		}

src/jrd/scl.epp

@@ -1224,6 +1224,7 @@ SecurityClass* SCL_recompute_class(thread_db* tdbb, const TEXT* string)
 		return NULL;
 
 	s_class->scl_flags = compute_access(tdbb, s_class, NULL, 0, NULL);
+	s_class->scl_blb_access = SecurityClass::BA_UNKNOWN;
 
 	if (s_class->scl_flags & SCL_exists)
 		return s_class;

src/jrd/scl.h

@@ -45,15 +45,16 @@ const size_t ACL_BLOB_BUFFER_SIZE = MAX_USHORT; // used to read/write acl blob
 class SecurityClass
 {
 public:
-    typedef ULONG flags_t;
+	typedef ULONG flags_t;
+	enum BlobAccessCheck { BA_UNKNOWN, BA_SUCCESS, BA_FAILURE };
 
 	SecurityClass(Firebird::MemoryPool &pool, const Firebird::MetaName& name)
-		: scl_flags(0), scl_name(pool, name), scl_blb_access(false)
+		: scl_flags(0), scl_name(pool, name), scl_blb_access(BA_UNKNOWN)
 	{}
 
 	flags_t scl_flags;			// Access permissions
 	const Firebird::MetaName scl_name;
-	bool scl_blb_access;
+	BlobAccessCheck scl_blb_access;
 
 	static const Firebird::MetaName& generate(const void*, const SecurityClass* item)
 	{

src/jrd/tra.cpp

@@ -3751,7 +3751,7 @@ void jrd_tra::rollforwardSavepoint(thread_db* tdbb)
 	}
 }
 
-void jrd_tra::checkBlob(thread_db* tdbb, const bid* blob_id)
+void jrd_tra::checkBlob(thread_db* tdbb, const bid* blob_id, jrd_fld* fld, bool punt)
 {
 	USHORT rel_id = blob_id->bid_internal.bid_relation_id;
 
@@ -3763,23 +3763,77 @@ void jrd_tra::checkBlob(thread_db* tdbb, const bid* blob_id)
 		return;
 	}
 
-	if (!tra_blobs->locate(blob_id->bid_temp_id()))
+	if (!tra_blobs->locate(blob_id->bid_temp_id()) &&
+		!tra_fetched_blobs.locate(*blob_id))
 	{
 		vec<jrd_rel*>* vector = tra_attachment->att_relations;
 		jrd_rel* blb_relation;
 
 		if (rel_id < vector->count() &&	(blb_relation = (*vector)[rel_id]))
 		{
-			if (blb_relation->rel_security_name.isEmpty())
+			const MetaName security_name = fld ?
+				fld->fld_security_name : blb_relation->rel_security_name;
+
+			if (security_name.isEmpty())
 				MET_scan_relation(tdbb, blb_relation);
 
-			SecurityClass* s_class = SCL_get_class(tdbb, blb_relation->rel_security_name.c_str());
+			SecurityClass* s_class = SCL_get_class(tdbb, security_name.c_str());
 
-			if (s_class && !s_class->scl_blb_access)
+			if (!s_class)
+				return;
+
+			switch (s_class->scl_blb_access)
 			{
-				SCL_check_access(tdbb, s_class, 0, 0, NULL, SCL_select, SCL_object_table, false,
+			case SecurityClass::BA_UNKNOWN:
-					blb_relation->rel_name);
+				// Relation (or field) has not been checked for access rights
-				s_class->scl_blb_access = true;
+				try
+				{
+					ThreadStatusGuard status_vector(tdbb);
+
+					if (fld)
+						SCL_check_access(tdbb, s_class, 0, 0, NULL, SCL_select, SCL_object_column,
+							false, fld->fld_name, blb_relation->rel_name);
+					else
+						SCL_check_access(tdbb, s_class, 0, 0, NULL, SCL_select, SCL_object_table,
+							false, blb_relation->rel_name);
+
+					s_class->scl_blb_access = SecurityClass::BA_SUCCESS;
+				}
+				catch (const Exception& ex)
+				{
+					StaticStatusVector status;
+					ex.stuffException(status);
+					if (status[1] != isc_no_priv)
+						throw;
+
+					// We don't have access to this relation
+					s_class->scl_blb_access = SecurityClass::BA_FAILURE;
+
+					if (punt)
+						throw;
+
+					// but someone else has (SP, view)
+					// store Blob ID as allowed in this transaction
+					tra_fetched_blobs.add(*blob_id);
+				}
+				break;
+
+			case SecurityClass::BA_FAILURE:
+				// Relation has been checked earlier and check was failed
+				if (punt)
+					ERR_post(Arg::Gds(isc_no_priv) << Arg::Str("SELECT") <<
+						(fld ? Arg::Str("COLUMN") : Arg::Str("TABLE")) <<
+						(fld ? Arg::Str(fld->fld_name) : Arg::Str(blb_relation->rel_name)));
+				else
+					tra_fetched_blobs.add(*blob_id);
+				break;
+
+			case SecurityClass::BA_SUCCESS:
+				// do nothing
+				break;
+
+			default:
+				fb_assert(false);
 			}
 		}
 	}

src/jrd/tra.h

@@ -103,6 +103,7 @@ struct BlobIndex
 };
 
 typedef Firebird::BePlusTree<BlobIndex, ULONG, MemoryPool, BlobIndex> BlobIndexTree;
+typedef Firebird::BePlusTree<bid, bid, MemoryPool> FetchedBlobIdTree;
 
 // Transaction block
 
@@ -167,6 +168,7 @@ public:
 		tra_memory_stats(parent_stats),
 		tra_blobs_tree(p),
 		tra_blobs(outer ? outer->tra_blobs : &tra_blobs_tree),
+		tra_fetched_blobs(p),
 		tra_arrays(NULL),
 		tra_deferred_job(NULL),
 		tra_resources(*p),
@@ -258,6 +260,7 @@ public:
 	Firebird::MemoryStats	tra_memory_stats;
 	BlobIndexTree tra_blobs_tree;		// list of active blobs
 	BlobIndexTree* tra_blobs;			// pointer to actual list of active blobs
+	FetchedBlobIdTree tra_fetched_blobs;	// list of fetched blobs
 	ArrayField*	tra_arrays;				// Linked list of active arrays
 	Lock*		tra_lock;				// lock for transaction
 	Lock*		tra_alter_db_lock;		// lock for ALTER DATABASE statement(s)
@@ -377,7 +380,7 @@ public:
 	void rollbackToSavepoint(thread_db* tdbb, SavNumber number);
 	void rollforwardSavepoint(thread_db* tdbb);
 	DbCreatorsList* getDbCreatorsList();
-	void checkBlob(thread_db* tdbb, const bid* blob_id);
+	void checkBlob(thread_db* tdbb, const bid* blob_id, jrd_fld* fld, bool punt);
 
 	GenIdCache* getGenIdCache()
 	{