Skip to content

Commit e40494a

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 7681865 commit e40494a

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,
@@ -277,8 +278,14 @@ async function validate_s3_policy(policy, bucket_name, get_account_handler) {
277278
throw new RpcError('MALFORMED_POLICY', 'Invalid principal in policy', { detail: statement.Principal });
278279
}
279280
for (const resource of _.flatten([statement.Resource || statement.NotResource])) {
281+
console.log(`************* VINAYAK RESOURCE = ${resource}`);
280282
const resource_bucket_part = resource.split('/')[0];
281-
const resource_regex = RegExp(`^${resource_bucket_part.replace(qm_regex, '.?').replace(ar_regex, '.*')}$`);
283+
const resource_regex = RegExp(
284+
`^${resource_bucket_part
285+
.replace(esc_regex, '\\$&')
286+
.replace(qm_regex, '.?')
287+
.replace(ar_regex, '.*')}$`
288+
);
282289
if (!resource_regex.test('arn:aws:s3:::' + bucket_name)) {
283290
throw new RpcError('MALFORMED_POLICY', 'Policy has invalid resource', { detail: resource });
284291
}

src/test/unit_tests/test_bucketspace_fs.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,27 @@ mocha.describe('bucketspace_fs', function() {
857857
}
858858
});
859859

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

0 commit comments

Comments
 (0)