Skip to content

Commit e0128aa

Browse files
committed
fix: prevent permission cycles
1 parent e482b00 commit e0128aa

File tree

3 files changed

+99
-3
lines changed

3 files changed

+99
-3
lines changed

src/backend/src/services/auth/PermissionService.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323

2424
const { get_user, get_app } = require("../../helpers");
2525
const { AssignableMethodsFeature } = require("../../traits/AssignableMethodsFeature");
26+
const { remove_paths_through_user } = require("../../unstructured/permission-scan-lib");
2627
const { Context } = require("../../util/context");
2728
const { get_a_letter, cylog } = require("../../util/debugutil");
2829
const BaseService = require("../BaseService");
@@ -180,6 +181,7 @@ class PermissionUtil {
180181
});
181182
}
182183
if ( finding.$ === 'path' ) {
184+
if ( finding.has_terminal === false ) continue;
183185
const new_extras = ( finding.data ) ? [
184186
finding.data,
185187
...extras,
@@ -223,8 +225,14 @@ class PermissionService extends BaseService {
223225
return options.length > 0;
224226
}
225227

226-
async scan (actor, permission_options) {
228+
async scan (actor, permission_options, _reserved, state) {
227229
const reading = [];
230+
231+
if ( ! state ) {
232+
state = {
233+
anti_cycle_actors: [actor],
234+
};
235+
}
228236

229237
if ( ! Array.isArray(permission_options) ) {
230238
permission_options = [permission_options];
@@ -240,6 +248,7 @@ class PermissionService extends BaseService {
240248
actor,
241249
permission_options,
242250
reading,
251+
state,
243252
});
244253
const end_ts = Date.now();
245254

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* Filters a permission reading so that it does not contain paths through the
3+
* specified user. This operation is performed recursively on all paths in the
4+
* reading.
5+
*
6+
* This does not prevent all possible cycles. To prevent all cycles, this filter
7+
* must by applied on each reading for a permission holder, specifying the
8+
* permission issuer as the user to filter out.
9+
*/
10+
const remove_paths_through_user = ({ reading, user }) => {
11+
const no_cycle_reading = [];
12+
13+
for ( let i = 0 ; i < reading.length ; i++ ) {
14+
const node = reading[i];
15+
16+
console.log('checking node...', node);
17+
18+
if ( node.$ === 'path' ) {
19+
if (
20+
node.issuer_username === user.username
21+
) {
22+
console.log('filtered out one');
23+
// process.exit(0);
24+
continue;
25+
}
26+
27+
node.reading = remove_paths_through_user({
28+
reading: node.reading,
29+
user,
30+
});
31+
}
32+
33+
no_cycle_reading.push(node);
34+
}
35+
36+
console.log('\x1B[36;1m ====', reading.length - no_cycle_reading.length, 'nodes filtered out ====\x1B[0m');
37+
38+
return no_cycle_reading;
39+
};
40+
41+
const reading_has_terminal = ({ reading }) => {
42+
for ( let i = 0 ; i < reading.length ; i++ ) {
43+
const node = reading[i];
44+
if ( node.has_terminal ) {
45+
return true;
46+
}
47+
if ( node.$ === 'option' ) {
48+
return true;
49+
}
50+
}
51+
52+
return false;
53+
};
54+
55+
module.exports = {
56+
remove_paths_through_user,
57+
reading_has_terminal,
58+
};

src/backend/src/unstructured/permission-scanners.js

+31-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
} = require("../data/hardcoded-permissions");
66
const { get_user } = require("../helpers");
77
const { Actor, UserActorType, AppUnderUserActorType } = require("../services/auth/Actor");
8+
const { reading_has_terminal } = require("./permission-scan-lib");
89

910
/*
1011
OPTIMAL FOLD LEVEL: 3
@@ -46,7 +47,7 @@ const PERMISSION_SCANNERS = [
4647
{
4748
name: 'user-user',
4849
async scan (a) {
49-
const { reading, actor, permission_options } = a.values();
50+
const { reading, actor, permission_options, state } = a.values();
5051
if ( !(actor.type instanceof UserActorType) ) {
5152
return;
5253
}
@@ -85,11 +86,26 @@ const PERMISSION_SCANNERS = [
8586
}),
8687
});
8788

89+
let should_continue = false;
90+
for ( const seen_actor of state.anti_cycle_actors ) {
91+
if ( seen_actor.type.user.id === issuer_actor.type.user.id ) {
92+
should_continue = true;
93+
break;
94+
}
95+
}
96+
97+
if ( should_continue ) continue;
98+
8899
// const issuer_perm = await this.check(issuer_actor, row.permission);
89-
const issuer_reading = await a.icall('scan', issuer_actor, row.permission);
100+
const issuer_reading = await a.icall(
101+
'scan', issuer_actor, row.permission, undefined, state);
102+
103+
const has_terminal = reading_has_terminal({ reading: issuer_reading });
104+
90105
reading.push({
91106
$: 'path',
92107
via: 'user',
108+
has_terminal,
93109
permission: row.permission,
94110
data: row.extra,
95111
holder_username: actor.type.user.username,
@@ -134,9 +150,13 @@ const PERMISSION_SCANNERS = [
134150
if ( ! issuer_group.hasOwnProperty(permission) ) continue;
135151
const issuer_reading =
136152
await a.icall('scan', issuer_actor, permission)
153+
154+
const has_terminal = reading_has_terminal({ reading: issuer_reading });
155+
137156
reading.push({
138157
$: 'path',
139158
via: 'hc-user-group',
159+
has_terminal,
140160
permission,
141161
data: issuer_group[permission],
142162
holder_username: actor.type.user.username,
@@ -188,9 +208,12 @@ const PERMISSION_SCANNERS = [
188208

189209
const issuer_reading = await a.icall('scan', issuer_actor, row.permission);
190210

211+
const has_terminal = reading_has_terminal({ reading: issuer_reading });
212+
191213
reading.push({
192214
$: 'path',
193215
via: 'user-group',
216+
has_terminal,
194217
// issuer: issuer_actor,
195218
permission: row.permission,
196219
data: row.extra,
@@ -240,6 +263,8 @@ const PERMISSION_SCANNERS = [
240263

241264
const issuer_actor = actor.get_related_actor(UserActorType);
242265
const issuer_reading = await a.icall('scan', issuer_actor, permission_options);
266+
267+
const has_terminal = reading_has_terminal({ reading: issuer_reading });
243268

244269
for ( const permission of permission_options ) {
245270
{
@@ -249,6 +274,7 @@ const PERMISSION_SCANNERS = [
249274
reading.push({
250275
$: 'path',
251276
permission,
277+
has_terminal,
252278
source: 'user-app-implied',
253279
by: 'user-app-hc-1',
254280
data: implied,
@@ -267,6 +293,7 @@ const PERMISSION_SCANNERS = [
267293
reading.push({
268294
$: 'path',
269295
permission,
296+
has_terminal,
270297
source: 'user-app-implied',
271298
by: 'user-app-hc-2',
272299
data: implicit_permissions[permission],
@@ -301,10 +328,12 @@ const PERMISSION_SCANNERS = [
301328
})();
302329
const issuer_actor = actor.get_related_actor(UserActorType);
303330
const issuer_reading = await a.icall('scan', issuer_actor, row.permission);
331+
const has_terminal = reading_has_terminal({ reading: issuer_reading });
304332
reading.push({
305333
$: 'path',
306334
via: 'user-app',
307335
permission: row.permission,
336+
has_terminal,
308337
data: row.extra,
309338
issuer_username: actor.type.user.username,
310339
reading: issuer_reading,

0 commit comments

Comments
 (0)