Changeset: 11a67b229142 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=11a67b229142
Added Files:
sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.py
sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.err
sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.out
Modified Files:
sql/server/rel_updates.c
sql/test/BugTracker-2020/Tests/All
sql/test/astro/Tests/update.sql
sql/test/astro/Tests/update.stable.out
sql/test/mapi/Tests/sql_int128.stable.out.int128
Branch: Oct2020
Log Message:
Added test and fix for bug 7035, ie dont be too aggressive on select privileges
on update queries. Reduce column visibility likewise in read queries
diffs (truncated from 451 to 300 lines):
diff --git a/sql/server/rel_updates.c b/sql/server/rel_updates.c
--- a/sql/server/rel_updates.c
+++ b/sql/server/rel_updates.c
@@ -1075,11 +1075,23 @@ update_table(sql_query *query, dlist *qn
return sql_error(sql, 02, SQLSTATE(3F000) "UPDATE: no such
schema '%s'", sname);
t = find_table_on_scope(sql, &s, sname, tname);
if (update_allowed(sql, t, tname, "UPDATE", "update", 0) != NULL) {
- sql_rel *r = NULL, *bt = rel_basetable(sql, t, alias ? alias :
tname), *res = bt;
+ sql_rel *r = NULL, *res = rel_basetable(sql, t, alias ? alias :
tname);
+ /* We have always to reduce the column visibility because of
the SET clause */
+ if (!table_privs(sql, t, PRIV_SELECT)) {
+ sql_rel *nres = NULL;
+ if (!(nres = rel_reduce_on_column_privileges(sql, res,
t)) && opt_where) /* on global updates the user may be able to upd*/
+ return sql_error(sql, 02, SQLSTATE(42000)
"UPDATE: insufficient privileges for user '%s' to update table '%s'",
sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql, "sys"),
"current_user")), tname);
+ if (!nres) {
+ res->exps = sa_list(sql->sa); /* hasn't select
privilege on any column, add just TID column to the list */
+ } else {
+ res = nres; /* add TID column to the columns it
has permission to */
+ }
+ list_append(res->exps, exp_column(sql->sa, alias ?
alias : tname, TID, sql_bind_localtype("oid"), CARD_MULTI, 0, 1));
+ }
if (opt_from) {
dlist *fl = opt_from->data.lval;
- list *refs = list_append(new_exp_list(sql->sa), (char*)
rel_name(bt));
+ list *refs = list_append(new_exp_list(sql->sa), (char*)
rel_name(res));
for (dnode *n = fl->h; n && res; n = n->next) {
sql_rel *fnd = table_ref(query, NULL,
n->data.sym, 0, refs);
@@ -1093,8 +1105,6 @@ update_table(sql_query *query, dlist *qn
return NULL;
}
if (opt_where) {
- if (!table_privs(sql, t, PRIV_SELECT))
- return sql_error(sql, 02, SQLSTATE(42000)
"UPDATE: insufficient privileges for user '%s' to update table '%s'",
sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql, "sys"),
"current_user")), tname);
if (!(r = rel_logical_exp(query, res, opt_where,
sql_where)))
return NULL;
/* handle join */
@@ -1107,7 +1117,7 @@ update_table(sql_query *query, dlist *qn
} else { /* update all */
r = res;
}
- return update_generate_assignments(query, t, r, bt,
assignmentlist, "UPDATE");
+ return update_generate_assignments(query, t, r,
rel_basetable(sql, t, alias ? alias : tname), assignmentlist, "UPDATE");
}
return NULL;
}
@@ -1158,8 +1168,11 @@ delete_table(sql_query *query, dlist *qn
if (opt_where) {
sql_exp *e;
- if (!table_privs(sql, t, PRIV_SELECT))
- return sql_error(sql, 02, SQLSTATE(42000)
"DELETE FROM: insufficient privileges for user '%s' to delete from table '%s'",
sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql, "sys"),
"current_user")), tname);
+ if (!table_privs(sql, t, PRIV_SELECT)) {
+ if (!(r = rel_reduce_on_column_privileges(sql,
r, t)))
+ return sql_error(sql, 02,
SQLSTATE(42000) "DELETE FROM: insufficient privileges for user '%s' to delete
from table '%s'", sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql,
"sys"), "current_user")), tname);
+ list_append(r->exps, exp_column(sql->sa, alias
? alias : tname, TID, sql_bind_localtype("oid"), CARD_MULTI, 0, 1));
+ }
if (!(r = rel_logical_exp(query, r, opt_where,
sql_where)))
return NULL;
e = exp_column(sql->sa, rel_name(r), TID,
sql_bind_localtype("oid"), CARD_MULTI, 0, 1);
@@ -1255,13 +1268,16 @@ merge_into_table(sql_query *query, dlist
return sql_error(sql, 02, SQLSTATE(3F000) "MERGE: no such
schema '%s'", sname);
if (!(t = find_table_on_scope(sql, &s, sname, tname)))
return sql_error(sql, 02, SQLSTATE(42S02) "MERGE: no such table
'%s'", tname);
- if (!table_privs(sql, t, PRIV_SELECT))
- return sql_error(sql, 02, SQLSTATE(42000) "MERGE: access denied
for %s to table %s%s%s'%s'",
-
sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql, "sys"),
"current_user")), t->s ? "'":"", t->s ? t->s->base.name : "", t->s ? "'.":"",
tname);
if (isMergeTable(t))
return sql_error(sql, 02, SQLSTATE(42000) "MERGE: merge
statements not available for merge tables yet");
bt = rel_basetable(sql, t, alias ? alias : tname);
+ if (!table_privs(sql, t, PRIV_SELECT)) {
+ if (!(bt = rel_reduce_on_column_privileges(sql, bt, t)))
+ return sql_error(sql, 02, SQLSTATE(42000) "MERGE:
access denied for %s to table %s%s%s'%s'",
+
sqlvar_get_string(find_global_var(sql, mvc_bind_schema(sql, "sys"),
"current_user")), t->s ? "'":"", t->s ? t->s->base.name : "", t->s ? "'.":"",
tname);
+ list_append(bt->exps, exp_column(sql->sa, alias ? alias :
tname, TID, sql_bind_localtype("oid"), CARD_MULTI, 0, 1));
+ }
joined = table_ref(query, NULL, tref, 0, NULL);
if (!bt || !joined)
return NULL;
@@ -1305,7 +1321,7 @@ merge_into_table(sql_query *query, dlist
extra_project->exps =
list_merge(extra_project->exps, rel_projections(sql, joined, NULL, 1, 0),
(fdup)NULL);
list_prepend(extra_project->exps,
exp_column(sql->sa, bt_name, TID, sql_bind_localtype("oid"), CARD_MULTI, 0, 1));
- upd_del = update_generate_assignments(query, t,
extra_project, rel_dup(bt), sts->h->data.lval, "MERGE");
+ upd_del = update_generate_assignments(query, t,
extra_project, rel_basetable(sql, t, bt_name), sts->h->data.lval, "MERGE");
} else if (uptdel == SQL_DELETE) {
if (!update_allowed(sql, t, tname, "MERGE",
"delete", 1))
return NULL;
@@ -1322,7 +1338,7 @@ merge_into_table(sql_query *query, dlist
extra_project = rel_project(sql->sa, join_rel,
rel_projections(sql, bt, NULL, 1, 0));
list_prepend(extra_project->exps,
exp_column(sql->sa, bt_name, TID, sql_bind_localtype("oid"), CARD_MULTI, 0, 1));
- upd_del = rel_delete(sql->sa, rel_dup(bt),
extra_project);
+ upd_del = rel_delete(sql->sa,
rel_basetable(sql, t, bt_name), extra_project);
} else {
assert(0);
}
@@ -1351,7 +1367,7 @@ merge_into_table(sql_query *query, dlist
if (!(insert = merge_generate_inserts(query, t,
extra_project, sts->h->data.lval, sts->h->next->data.sym)))
return NULL;
- if (!(insert = rel_insert(query->sql, rel_dup(bt),
insert)))
+ if (!(insert = rel_insert(query->sql,
rel_basetable(sql, t, bt_name), insert)))
return NULL;
} else {
assert(0);
diff --git a/sql/test/BugTracker-2020/Tests/All
b/sql/test/BugTracker-2020/Tests/All
--- a/sql/test/BugTracker-2020/Tests/All
+++ b/sql/test/BugTracker-2020/Tests/All
@@ -41,3 +41,4 @@ view_with_aggr_column.Bug-7023
delete-transaction-loose-inserts.Bug-7024
revokeRoleUserLoggedIN.Bug-7026
drop-table-with-auto_increment.Bug-7030
+user-update-privs.Bug-7035
diff --git a/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.py
b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.py
new file mode 100644
--- /dev/null
+++ b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.py
@@ -0,0 +1,67 @@
+import os, sys, pymonetdb
+
+
+port = int(os.environ['MAPIPORT'])
+db = os.environ['TSTDB']
+
+conn1 =
pymonetdb.connect(port=port,database=db,autocommit=True,username='monetdb',password='monetdb')
+cur1 = conn1.cursor()
+cur1.execute("""
+start transaction;
+create table t (a int, b int);
+create user usr with password 'usr' name 'usr' schema sys;
+grant select(a) on t to usr;
+grant update (b) on t to usr;
+grant delete on t to usr;
+commit;
+""")
+cur1.close()
+conn1.close()
+
+conn1 =
pymonetdb.connect(port=port,database=db,autocommit=True,username='usr',password='usr')
+cur1 = conn1.cursor()
+cur1.execute('select a from t;')
+if cur1.fetchall() != []:
+ sys.stderr.write("[] expected")
+if cur1.execute('update t set b = 23;') != 0:
+ sys.stderr.write("0 rows affected expected")
+if cur1.execute('update t set b = 32 where a = 6;') != 0:
+ sys.stderr.write("0 rows affected expected")
+if cur1.execute('update t set b = a;') != 0:
+ sys.stderr.write("0 rows affected expected")
+if cur1.execute('delete from t;') != 0:
+ sys.stderr.write("0 rows affected expected")
+if cur1.execute('delete from t where a = 2;') != 0:
+ sys.stderr.write("0 rows affected expected")
+try:
+ cur1.execute('update t set b = b;') # error, not allowed
+ sys.stderr.write("Exception expected")
+except pymonetdb.DatabaseError as e:
+ if "SELECT: identifier 'b' unknown" not in str(e):
+ sys.stderr.write('Wrong error %s, expected SELECT: identifier \'b\'
unknown' % (str(e)))
+try:
+ cur1.execute('update t set b = 1 where b = 5;') # error, not allowed
+ sys.stderr.write("Exception expected")
+except pymonetdb.DatabaseError as e:
+ if "SELECT: identifier 'b' unknown" not in str(e):
+ sys.stderr.write('Wrong error %s, expected SELECT: identifier \'b\'
unknown' % (str(e)))
+try:
+ cur1.execute('delete from t where b = 5;') # error, not allowed
+ sys.stderr.write("Exception expected")
+except pymonetdb.DatabaseError as e:
+ if "SELECT: identifier 'b' unknown" not in str(e):
+ sys.stderr.write('Wrong error %s, expected SELECT: identifier \'b\'
unknown' % (str(e)))
+
+cur1.close()
+conn1.close()
+
+conn1 =
pymonetdb.connect(port=port,database=db,autocommit=True,username='monetdb',password='monetdb')
+cur1 = conn1.cursor()
+cur1.execute("""
+start transaction;
+drop user usr;
+drop table t;
+commit;
+""")
+cur1.close()
+conn1.close()
diff --git
a/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.err
b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.err
new file mode 100644
--- /dev/null
+++ b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.err
@@ -0,0 +1,12 @@
+stderr of test 'user-update-privs.Bug-7035` in directory
'sql/test/BugTracker-2020` itself:
+
+
+# 16:38:34 >
+# 16:38:34 > "/usr/bin/python3.9" "user-update-privs.Bug-7035.py"
"user-update-privs.Bug-7035"
+# 16:38:34 >
+
+
+# 16:38:34 >
+# 16:38:34 > "Done."
+# 16:38:34 >
+
diff --git
a/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.out
b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.out
new file mode 100644
--- /dev/null
+++ b/sql/test/BugTracker-2020/Tests/user-update-privs.Bug-7035.stable.out
@@ -0,0 +1,12 @@
+stdout of test 'user-update-privs.Bug-7035` in directory
'sql/test/BugTracker-2020` itself:
+
+
+# 16:38:34 >
+# 16:38:34 > "/usr/bin/python3.9" "user-update-privs.Bug-7035.py"
"user-update-privs.Bug-7035"
+# 16:38:34 >
+
+
+# 16:38:34 >
+# 16:38:34 > "Done."
+# 16:38:34 >
+
diff --git a/sql/test/astro/Tests/update.sql b/sql/test/astro/Tests/update.sql
--- a/sql/test/astro/Tests/update.sql
+++ b/sql/test/astro/Tests/update.sql
@@ -32,3 +32,37 @@ UPDATE fluxz
AND cm_flux.filter = fluxz.filter
)
;
+
+-- Run the update to make sure it doesn't crash
+UPDATE fluxz
+ SET (filter
+ ,f_datapoints
+ ,avg_flux
+ ,avg_fluxsq
+ ,avg_w
+ ,avg_wflux
+ ,avg_wfluxsq
+ ,avg_dec_zone_deg
+ )
+ =
+ (SELECT filter
+ ,f_datapoints
+ ,avg_flux
+ ,avg_fluxsq
+ ,avg_w
+ ,avg_wflux
+ ,avg_wfluxsq
+ ,avg_dec_zone_deg
+ FROM cm_flux
+ WHERE cm_flux.runcat = fluxz.runcat
+ AND cm_flux.active = TRUE
+ AND cm_flux.filter = 'g'
+ AND cm_flux.filter = fluxz.filter
+ )
+ WHERE EXISTS (SELECT runcat
+ FROM cm_flux
+ WHERE cm_flux.runcat = fluxz.runcat
+ AND cm_flux.active = TRUE
+ AND cm_flux.filter = 'g'
+ AND cm_flux.filter = fluxz.filter
+ );
diff --git a/sql/test/astro/Tests/update.stable.out
b/sql/test/astro/Tests/update.stable.out
--- a/sql/test/astro/Tests/update.stable.out
+++ b/sql/test/astro/Tests/update.stable.out
@@ -55,7 +55,7 @@ update(
| single project (
| | single left outer join (
| | | semijoin (
-| | | | table("sys"."fluxz") [ "fluxz"."runcat" NOT NULL, "fluxz"."filter" NOT
NULL, "fluxz"."f_datapoints" NOT NULL, "fluxz"."active" NOT NULL,
"fluxz"."avg_flux" NOT NULL, "fluxz"."avg_fluxsq" NOT NULL, "fluxz"."avg_w" NOT
NULL, "fluxz"."avg_wflux" NOT NULL, "fluxz"."avg_wfluxsq" NOT NULL,
"fluxz"."avg_dec_zone_deg" NOT NULL, "fluxz"."%TID%" NOT NULL ] COUNT ,
+| | | | table("sys"."fluxz") [ "fluxz"."runcat" NOT NULL, "fluxz"."filter" NOT
NULL, "fluxz"."%TID%" NOT NULL ] COUNT ,
| | | | select (
| | | | | table("sys"."cm_flux") [ "cm_flux"."runcat" NOT NULL,
"cm_flux"."filter" NOT NULL, "cm_flux"."active" NOT NULL ] COUNT
| | | | ) [ "cm_flux"."active" NOT NULL = boolean "true", "cm_flux"."filter"
NOT NULL = char(1) "g" ]
@@ -68,6 +68,30 @@ update(
| | ) [ "cm_flux"."runcat" NOT NULL = "fluxz"."runcat" NOT NULL,
"cm_flux"."filter" NOT NULL = "fluxz"."filter" NOT NULL ]
| ) [ "fluxz"."%TID%" NOT NULL, "%1"."%1" NOT NULL as "fluxz"."filter",
"%2"."%2" NOT NULL as "fluxz"."f_datapoints", "%3"."%3" NOT NULL as
"fluxz"."avg_flux", "%4"."%4" NOT NULL as "fluxz"."avg_fluxsq", "%5"."%5" NOT
NULL as "fluxz"."avg_w", "%6"."%6" NOT NULL as "fluxz"."avg_wflux", "%7"."%7"
NOT NULL as "fluxz"."avg_wfluxsq", "%10"."%10" NOT NULL as
"fluxz"."avg_dec_zone_deg" ]
) [ "fluxz"."%TID%" NOT NULL, "fluxz"."filter" NOT NULL,
"fluxz"."f_datapoints" NOT NULL, "fluxz"."avg_flux" NOT NULL,
"fluxz"."avg_fluxsq" NOT NULL, "fluxz"."avg_w" NOT NULL, "fluxz"."avg_wflux"
NOT NULL, "fluxz"."avg_wfluxsq" NOT NULL, "fluxz"."avg_dec_zone_deg" NOT NULL ]
+#UPDATE fluxz
+# SET (filter
+# ,f_datapoints
+# ,avg_flux
+# ,avg_fluxsq
+# ,avg_w
+# ,avg_wflux
+# ,avg_wfluxsq
+# ,avg_dec_zone_deg
+# )
+# =
+# (SELECT filter
+# ,f_datapoints
+# ,avg_flux
+# ,avg_fluxsq
+# ,avg_w
+# ,avg_wflux
+# ,avg_wfluxsq
+# ,avg_dec_zone_deg
+# FROM cm_flux
+# WHERE cm_flux.runcat = fluxz.runcat
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
