From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 201A22AD38 for ; Fri, 22 Mar 2019 05:29:03 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zvbhmbWKmdox for ; Fri, 22 Mar 2019 05:29:03 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CB2252AD37 for ; Fri, 22 Mar 2019 05:29:02 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 2/9] box: fix _trigger and _ck_constraint access check References: <3226bb82065bbaea578a909f5382571bbe01fa3b.1548838034.git.kshcherbatov@tarantool.org> <29CE984E-A6A6-4E1D-AFFD-721477BD73B1@tarantool.org> From: Vladislav Shpilevoy Message-ID: <410708e1-331b-fd3a-ab61-459ebbf82ceb@tarantool.org> Date: Fri, 22 Mar 2019 12:29:00 +0300 MIME-Version: 1.0 In-Reply-To: <29CE984E-A6A6-4E1D-AFFD-721477BD73B1@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, "n.pettik" Cc: Kirill Shcherbatov I do not see fixes for the comments below. On 11/03/2019 22:29, n.pettik wrote: > >> src/box/alter.cc | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/src/box/alter.cc b/src/box/alter.cc >> index 0589c9678..ab3dd2e22 100644 >> --- a/src/box/alter.cc >> +++ b/src/box/alter.cc >> @@ -3521,6 +3521,11 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) >> uint32_t space_id = >> tuple_field_u32_xc(old_tuple, >> BOX_TRIGGER_FIELD_SPACE_ID); >> + struct space *space = space_by_id(space_id); >> + assert(space != NULL); >> + access_check_ddl(space->def->name, space->def->id, >> + space->def->uid, SC_SPACE, PRIV_A); > > Why did you check only alter privilege? On drop we should check > drop privilege, on alter - alter privilege and on creation - creation > privilege. > > This particular branch processes drop case, so we should use PRIV_D. > >> + >> char *trigger_name = >> (char *)region_alloc_xc(&fiber()->gc, >> trigger_name_len + 1); >> @@ -3574,6 +3579,10 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) >> "trigger space_id does not match the value " >> "resolved on AST building from SQL"); >> } >> + struct space *space = space_by_id(space_id); >> + assert(space != NULL); >> + access_check_ddl(space->def->name, space->def->id, >> + space->def->uid, SC_SPACE, PRIV_A); > > And PRIV_C depending on situation. > > The same for _fk_constraint space. > >> @@ -4018,6 +4031,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) >> space_cache_find_xc(fk_def->child_id); >> struct space *parent_space = >> space_cache_find_xc(fk_def->parent_id); >> + access_check_ddl(child_space->def->name, child_space->def->id, >> + child_space->def->uid, SC_SPACE, PRIV_A); >> + access_check_ddl(parent_space->def->name, parent_space->def->id, >> + parent_space->def->uid, SC_SPACE, PRIV_A); >> struct fkey *old_fkey = >> fkey_grab_by_name(&child_space->child_fkey, >> fk_def->name); > > Please, add tests on these cases. > >