Skip to content

Commit ff54020

Browse files
committed
[MCG] Using put-bucket-policy with wrong syntax under Resource results in InternalError instead of MalformedPolicy
The malformed syntax should give malformed systax error. Issue: Square brackets ([ ]) in resource_bucket_part were misinterpreted in regex. Fix: Escape all regex special characters before inserting into RegExp(). Fixes: https://issues.redhat.com/browse/DFBUGS-1517 Signed-off-by: Vinayakswami Hariharmath <[email protected]>
1 parent 0669471 commit ff54020

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

src/endpoint/s3/s3_bucket_policy_utils.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ const OP_NAME_TO_ACTION = Object.freeze({
9191

9292
const qm_regex = /\?/g;
9393
const ar_regex = /\*/g;
94+
const esc_regex = /[-/^$+?.()|[\]{}]/g;
9495

9596
const predicate_map = {
9697
'StringEquals': (request_value, policy_value) => request_value === policy_value,
@@ -268,8 +269,14 @@ async function validate_s3_policy(policy, bucket_name, get_account_handler) {
268269
throw new RpcError('MALFORMED_POLICY', 'Invalid principal in policy', { detail: statement.Principal });
269270
}
270271
for (const resource of _.flatten([statement.Resource || statement.NotResource])) {
272+
console.log(`************* VINAYAK RESOURCE = ${resource}`);
271273
const resource_bucket_part = resource.split('/')[0];
272-
const resource_regex = RegExp(`^${resource_bucket_part.replace(qm_regex, '.?').replace(ar_regex, '.*')}$`);
274+
const resource_regex = RegExp(
275+
`^${resource_bucket_part
276+
.replace(esc_regex, '\\$&')
277+
.replace(qm_regex, '.?')
278+
.replace(ar_regex, '.*')}$`
279+
);
273280
if (!resource_regex.test('arn:aws:s3:::' + bucket_name)) {
274281
throw new RpcError('MALFORMED_POLICY', 'Policy has invalid resource', { detail: resource });
275282
}

src/test/unit_tests/test_bucketspace_fs.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,27 @@ mocha.describe('bucketspace_fs', function() {
852852
}
853853
});
854854

855+
mocha.it('put_bucket_policy, Wrong Resouce list syntax', async function() {
856+
const policy = {
857+
Version: '2012-10-17',
858+
Statement: [{
859+
Sid: 'id-22',
860+
Effect: 'Allow',
861+
Principal: { AWS: 'user10' },
862+
Action: ['s3:*'],
863+
Resource: "['arn:aws:s3:::*']"
864+
}]
865+
};
866+
const param = { name: test_bucket, policy: policy };
867+
try {
868+
await bucketspace_fs.put_bucket_policy(param);
869+
assert.fail('should have failed with invalid principal in policy');
870+
} catch (err) {
871+
assert.equal(err.rpc_code, 'MALFORMED_POLICY');
872+
assert.equal(err.message, 'Invalid principal in policy');
873+
}
874+
});
875+
855876
mocha.it('put_bucket_policy other account array', async function() {
856877
const policy = {
857878
Version: '2012-10-17',

0 commit comments

Comments
 (0)