Issue Details (XML | Word | Printable)

Key: CORE-3242
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Alexander Peshkov
Reporter: Ain Valtin
Votes: 0
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Firebird Core

Recursive stored procedure shouldnt require execute right to call itself

Created: 17/Nov/10 10:36 AM   Updated: 14/Jun/15 04:45 PM
Component/s: Engine, Security
Affects Version/s: 2.5.0
Fix Version/s: 3.0 Beta 1

Environment: WI-V2.5.0.26074 Firebird 2.5

Target: 3.0 Beta 1
QA Status: Done successfully


 Description  « Hide
Create these objects as some other user than "test":

CREATE TABLE TAB_Foo ( Fld INTEGER );

SET TERM ^ ;

CREATE PROCEDURE SP_R(ID INTEGER) AS
BEGIN
  IF(ID IS NULL AND ID IS NOT NULL)THEN EXECUTE PROCEDURE SP_R(ID);
END^

CREATE TRIGGER Foo_BI FOR TAB_Foo
ACTIVE BEFORE INSERT POSITION 15
AS
BEGIN
  EXECUTE PROCEDURE SP_R(NEW.Fld);
END^

SET TERM ; ^

GRANT ALL ON TAB_Foo TO test;
GRANT EXECUTE ON PROCEDURE SP_R TO TRIGGER Foo_BI;

connect as "test" and execute:

INSERT INTO TAB_Foo(Fld) VALUES(1);

Error: *** IBPP::SQLException ***
Context: Statement::Prepare( INSERT INTO TAB_Foo(Fld) VALUES(1) )
Message: isc_dsql_prepare failed

SQL Message : -551
This user does not have privilege to perform this operation on this object.

Engine Code : 335544352
Engine Message : no permission for execute access to PROCEDURE SP_R

Workaround is to grant execute right on SP_R to itself:
GRANT EXECUTE ON PROCEDURE SP_R TO PROCEDURE SP_R;

but this shouldn't really be necessary as whoever/whatever called SP_R must have right to do so, thus the SP should have right to recursively call itself without any excplicit grant.

 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Vlad Khorsun added a comment - 17/Nov/10 02:20 PM
> Workaround is to grant execute right on SP_R to itself:
> GRANT EXECUTE ON PROCEDURE SP_R TO PROCEDURE SP_R;

> but this shouldn't really be necessary as whoever/whatever called SP_R must have right to do so, thus the SP should have right to recursively call itself without any excplicit grant.


Why do you think so ? Any proof (from standard, for example) ?

Ain Valtin added a comment - 17/Nov/10 02:38 PM - edited
Vlad - I don't have any proof for that other than logic I already mentioned: if the "original caller" had right to invoke the prodcedure, then the procedure should have a right to recurse as the original caller has right to execute the procedure.
I can't see any advantage in the current implementation, only additional work for the DBA to maintain rights.

Sean Leyne added a comment - 17/Nov/10 04:56 PM
Ain,

While I would agree that the grants for recursive procedures might seem obvious, "obvious" is not a concept that the SQL committee/standard has ever embraced.

Further, adding a single grant for the SP to call itself can hardly be described as a significant effort for the DBA.

Alexander Peshkov added a comment - 18/Nov/10 07:41 AM
Ain, if we follow your logic in this example:

CREATE PROCEDURE SP1 AS
 BEGIN
   IF(la-la-la) THEN EXECUTE PROCEDURE SP2;
 END^

CREATE PROCEDURE SP2 AS
 BEGIN
   IF(bla-bla-bla) THEN EXECUTE PROCEDURE SP3;
 END^

CREATE PROCEDURE SP3 AS
 BEGIN
   IF(bla-bla-bla) THEN EXECUTE PROCEDURE SP2;
 END^

GRANT EXECUTE ON PROCEDURE SP2 TO PROCEDURE SP1;
GRANT EXECUTE ON PROCEDURE SP3 TO PROCEDURE SP2;
GRANT EXECUTE ON PROCEDURE SP1 TO USER TEST;

we must say that SP3 must be 'auto-granted' execute on SP2 cause otherwise initial call to SP2 will fail. I.e. each object in the trace of SP calls must add it's rights to the context of a call. I'm far not sure that this is correct from security POV.

Ain Valtin added a comment - 18/Nov/10 10:14 AM
Alexander, I see your point. However, in this case it seems obvious that SP3 must be granted right to execute SP2 (unless SP3 is only meant to be called indirectly, via SP2, in which case "cascading rights" would cover it).

I guess the question really boils down to what standard says about user rights in such a situation and/or how other major players have implemented it...

Alexander Peshkov added a comment - 18/Nov/10 02:37 PM
Standard says that something may be granted only to user or role. According to standard when procedure begins to execute security context of session is saved and security context of user, who owns procedure, is used _instead_. I.e. instead of "cascading rights" standard suggests "replacing rights".

This means that in my sample FB works according to standard, but in initial case self recursive call is always possible - any user always has full rights on the objects owned by him. But this is definitely security schema different from one used by FB. Now I'm unsure should we always allow self-call or not.

Dmitry Yemanov added a comment - 18/Nov/10 03:04 PM
I would vote for allowing recursive calls. We already have similar permission shortcuts, i.e. triggers have all access to their base relation (at the table level) and its fields (at the field level) without any explicit grants.

Ain Valtin added a comment - 18/Nov/10 06:21 PM
> Standard says that something may be granted only to user or role.

So granting rights to SP and trigger is FireBird's non-standard extension?


> According to standard when procedure begins to execute security context of session is saved and security context of user,
> who owns procedure, is used _instead_. I.e. instead of "cascading rights" standard suggests "replacing rights".

I personaly prefer FB's way - it gives more control over user rights.


> Now I'm unsure should we always allow self-call or not.

Could the current implementation viewed as useful feature? In my opinion no, so I would change it to always allow self-call.

Alexander Peshkov added a comment - 19/Nov/10 07:48 AM
Let's be decided with suggested solution - and therefore increase priority.

Simonov Denis added a comment - 21/Apr/14 02:22 PM - edited
Please, do the same for stored functions.

AS SYSDBA

set term ^;

create or alter function fn_rec(i int) returns int
as
begin
  if (i>1) then
    return i*fn_rec(i-1);
  else
    return i;
end^

create or alter function fn_fact(i int) returns int
as
begin
  return fn_rec(i);
end^

set term ;^

revoke execute on function fn_rec from test;
grant execute on function fn_rec to fn_fact;
grant execute on function fn_fact to test;

AS TEST

SQL> select fn_fact(10) from rdb$database;
Statement failed, SQLSTATE = 28000
no permission for EXECUTE access to FUNCTION FN_REC
SQL>

Alexander Peshkov added a comment - 21/Apr/14 05:07 PM
Denis, it will be great not to hurry so much when preparing samples - correct DDL is no
  grant execute on function fn_rec to fn_fact;
but
  grant execute on function fn_rec to function fn_fact;

Simonov Denis added a comment - 21/Apr/14 06:56 PM
I'm sorry.