Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Date: Mon, 23 Dec 2013 12:48:50 +0000 (UTC)
From: "Knut Anders Hatlen (JIRA)" <jira@apache.org>
To: derby-dev@db.apache.org
Message-ID: <JIRA.12683643.1386611369873.3824.1387802930933@arcas>
In-Reply-To: <JIRA.12683643.1386611369873@arcas>
References: <JIRA.12683643.1386611369873@arcas>
Subject: [jira] [Commented] (DERBY-6429) Privilege checks for UPDATE
 statements are wrong.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/DERBY-6429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13855607#comment-13855607 ] 

Knut Anders Hatlen commented on DERBY-6429:
-------------------------------------------

Thanks for looking into this bug, Rick. Your approach sounds good to me.

When I looked through the patch the first time, I was a little confused by some of the naming. Nothing major, but here's what confused me:

1) I had a hard time wrapping my head around the new concept of "visitable tags". Until I found out there was no such thing as a "visitable tag". :) It became much clearer to me when I learnt to read "addVisitableTag" as "add a tag to this visitable" instead of "add a visitable tag". Maybe dropping "visitable" (which is redundant, anyway, since the methods live in the Visitable interface) and calling the new methods "addTag" and "getTags" would be sufficient?

2) TagFilter.UPDATE_PRIVS is very similar to Authorizer.UPDATE_PRIV. However, the nodes that get tagged with UPDATE_PRIVS will actually be checked for SELECT_PRIV. Maybe the constant in TagFilter could be called something that more clearly states that it is for privilege checking of the UPDATE statement (as opposed to checking that the UPDATE privilege is granted).

Some more nits:

- TagFilter.accept() could have called visitableTags.contains(_tag) instead of doing it manually in a for loop.

- Maybe it would be a cleaner interface to have
  boolean hasTag(String tag)
  instead of the method that returns the list of tags in the Visitable interface. That would avoid exposing the internal representation through the interface method (which some static code analyzers tend to frown upon and flag as potential security warnings), and it would simplify two of the three callers of getVisitableTags(). (The third caller, StaticMethodCallNode, still needs the list of tags in order to copy it, but this could be done using a package-private method in QTN instead of via the interface.)

> Privilege checks for UPDATE statements are wrong.
> -------------------------------------------------
>
>                 Key: DERBY-6429
>                 URL: https://issues.apache.org/jira/browse/DERBY-6429
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 10.11.0.0
>            Reporter: Rick Hillegas
>            Assignee: Rick Hillegas
>         Attachments: derby-6429-01-ab-privilegeFilters.diff, derby-6429-01-ac-privilegeFilters.diff
>
>
> UPDATE statements confuse SELECT and UPDATE privileges. Consider the following SET clause:
>    SET updateColumn = selectColumn
> According to part 2 of the 2011 edition of the SQL Standard, that SET clause requires the following privileges:
> 1) UPDATE privilege on updateColumn. Privileges for the left side of a SET clause are described by section 14.14 (update statement: searched), access rule 1b.
> 2) SELECT privilege on selectColumn. Privileges for the right side of a SET clause are described by section 14.15 (set clause list) and the various productions underneath value expression. In this case, we have a column reference, whose privileges are governed by section 6.7 (column reference), access rule 2.
> However, Derby requires the following:
> 1') UPDATE privilege on both updateColumn and selectColumn
> When we address this bug, we should make corresponding changes to the MERGE statement.
> The following script shows the current behavior:
> connect 'jdbc:derby:memory:db;user=test_dbo;create=true';
> call syscs_util.syscs_create_user( 'TEST_DBO', 'test_dbopassword' );
> call syscs_util.syscs_create_user( 'RUTH', 'ruthpassword' );
> connect 'jdbc:derby:memory:db;shutdown=true';
> connect 'jdbc:derby:memory:db;user=test_dbo;password=test_dbopassword' as dbo;
> create table t1_025
> (
>     a int primary key,
>     updateColumn int,
>     selectColumn int,
>     privateColumn int
> );
> grant update ( updateColumn ) on t1_025 to ruth;
> grant select ( selectColumn ) on t1_025 to ruth;
> insert into t1_025 values ( 1, 100, 1000, 10000 );
> connect 'jdbc:derby:memory:db;user=ruth;password=ruthpassword' as ruth;
> -- correctly succeeds because ruth has UPDATE privilege on updateColumn
> update test_dbo.t1_025 set updateColumn = 17;
> -- the error message incorrectly states that the missing privilege
> -- is UPDATE privilege on privateColumn
> update test_dbo.t1_025 set updateColumn = privateColumn;
> -- incorrectly fails.
> -- ruth does have UPDATE privilege on updateColumn
> -- and SELECT privilege on selectColumn, which should be good enough.
> -- however, the error message incorrectly states that the missing privilege
> -- is UPDATE privilege on selectColumn.
> update test_dbo.t1_025 set updateColumn = selectColumn;
> -- incorrectly succeeds even though ruth does not have SELECT privilege on updateColumn
> update test_dbo.t1_025 set updateColumn = 2 * updateColumn;
> set connection dbo;
> select * from t1_025 order by a;


--
This message was sent by Atlassian JIRA
(v6.1.5#6160)