Skip to content

Commit aa3b486

Browse files
Fix - list_buckets allowing unauthorized bucket access
Signed-off-by: Aayush Chouhan <achouhan@redhat.com>
1 parent b83a047 commit aa3b486

File tree

2 files changed

+15
-3
lines changed

2 files changed

+15
-3
lines changed

src/server/common_services/auth_server.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,12 @@ function _get_auth_info(account, system, authorized_by, role, extra) {
536536
* @returns {Promise<boolean>} true if the account has permission to perform the action on the bucket
537537
*/
538538
async function has_bucket_action_permission(bucket, account, action, req_query, bucket_path = "") {
539+
if (!account || !account.email) {
540+
dbg.warn('has_bucket_action_permission: account or account.email is missing');
541+
return false;
542+
}
539543
dbg.log1('has_bucket_action_permission:', bucket.name, account.email, bucket.owner_account.email);
544+
540545
// If the system owner account wants to access the bucket, allow it
541546
if (bucket.system.owner.email.unwrap() === account.email.unwrap()) return true;
542547

@@ -558,7 +563,9 @@ async function has_bucket_action_permission(bucket, account, action, req_query,
558563
);
559564

560565
if (result === 'DENY') return false;
561-
return is_owner || result === 'ALLOW';
566+
if (result === 'ALLOW') return true;
567+
568+
return is_owner;
562569
}
563570

564571
/**

src/server/system_services/bucket_server.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,8 +1062,13 @@ async function list_buckets(req) {
10621062
let continuation_token = req.rpc_params?.continuation_token;
10631063
const max_buckets = req.rpc_params?.max_buckets;
10641064

1065-
const accessible_bucket_list = system_store.data.buckets.filter(
1066-
async bucket => await req.has_s3_bucket_permission(bucket, "s3:ListBucket", req) && !bucket.deleting
1065+
const accessible_bucket_list = [];
1066+
await Promise.all(
1067+
system_store.data.buckets.map(async bucket => {
1068+
if (bucket.deleting) return;
1069+
const has_permission = await req.has_s3_bucket_permission(bucket, "s3:ListBucket");
1070+
if (has_permission) accessible_bucket_list.push(bucket);
1071+
})
10671072
);
10681073

10691074
accessible_bucket_list.sort((a, b) => a.name.unwrap().localeCompare(b.name.unwrap()));

0 commit comments

Comments
 (0)