From 4a562d4ba9d67f0c379daad755ae5ed58569b68c Mon Sep 17 00:00:00 2001 From: AkhtarAmir <31914988+AkhtarAmir@users.noreply.github.com> Date: Thu, 6 Jan 2022 14:22:15 +0500 Subject: [PATCH] SLK-42343: Added condition key to be check for SQS Access Policy (#991) * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * Added vpcEndpointAcceptance plugin and spec file * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * Added plugin and spec file for launch wizard security groups * Refactored code in plaintextParameters plugin and spec file * SPLOIT-113: Updated custom settings * Made PR requested changes * SPLOIT-113: Added regex to check if NoEcho is enabled * Accommodated PR changes * Fixed eslint issues * Update exports.js * Fixed eslint issues * Update index.js * Update index.js * Added cloudformation in china and gov regions * Accomodated PR changes * Updated status in result of failure * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * Added plugin and spec file for launch wizard security groups * Added vpcEndpointAcceptance plugin and spec file * Refactored code in plaintextParameters plugin and spec file * SPLOIT-113: Updated custom settings * Made PR requested changes * SPLOIT-113: Added regex to check if NoEcho is enabled * Accommodated PR changes * Fixed eslint issues * Update index.js * Update index.js * Accomodated PR changes * Updated status in result of failure * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * Added plugin and spec file for launch wizard security groups * Added vpcEndpointAcceptance plugin and spec file * Refactored code in plaintextParameters plugin and spec file * SPLOIT-113: Updated custom settings * Made PR requested changes * SPLOIT-113: Added regex to check if NoEcho is enabled * Accommodated PR changes * Fixed eslint issues * Update index.js * Update index.js * Accomodated PR changes * Updated status in result of failure * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * SPLOIT-113: Added Plain Text Parameters plugin for CloudFormation * Added plugin and spec file for launch wizard security groups * Added vpcEndpointAcceptance plugin and spec file * Refactored code in plaintextParameters plugin and spec file * SPLOIT-113: Updated custom settings * Made PR requested changes * SPLOIT-113: Added regex to check if NoEcho is enabled * Accommodated PR changes * Fixed eslint issues * Update exports.js * Update index.js * Update index.js * Accomodated PR changes * Updated status in result of failure * Removed unnecesary rebase changes * Added superlinter * Added scans ci * Updated Ci file * Updated Node version in CI file * removed spech check command * Delete scan_ci.yml * Added spellcheck * HOTFIX/sqs-public-access: Logic will check policy condition as well * Fixed FN issue for S3 Bucket Secure Transport * Fixed PUb/Sub Topic Encryption error * Updated Topic Encryption plugin * Update functions.js * Added condition key to be check for SQS Access Policy --- plugins/aws/sqs/sqsCrossAccount.js | 54 +++++++++++++++++------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/plugins/aws/sqs/sqsCrossAccount.js b/plugins/aws/sqs/sqsCrossAccount.js index 99b719cf6f..7d2af7d676 100644 --- a/plugins/aws/sqs/sqsCrossAccount.js +++ b/plugins/aws/sqs/sqsCrossAccount.js @@ -85,7 +85,7 @@ module.exports = { return rcb(); } - async.each(listQueues.data, function(queue, cb){ + listQueues.data.forEach(queue => { var getQueueAttributes = helpers.addSource(cache, source, ['sqs', 'getQueueAttributes', region, queue]); @@ -97,8 +97,7 @@ module.exports = { helpers.addResult(results, 3, 'Unable to query SQS for queue: ' + queue, region); - - return cb(); + return; } var queueArn = getQueueAttributes.data.Attributes.QueueArn; @@ -107,7 +106,7 @@ module.exports = { helpers.addResult(results, 0, 'The SQS queue does not use a custom policy', region, queueArn); - return cb(); + return; } try { @@ -117,7 +116,7 @@ module.exports = { 'The SQS queue policy could not be parsed to valid JSON.', region, queueArn); - return cb(); + return; } var globalActions = []; @@ -126,18 +125,33 @@ module.exports = { var statements = helpers.normalizePolicyDocument(policy); for (var statement of statements) { - if (!statement.Effect || statement.Effect !== 'Allow') continue; - if (!statement.Principal) continue; + if (!statement.Effect || statement.Effect !== 'Allow' || !statement.Principal) continue; + + var crossAccountAccess = false; + var conditionalPrincipals = (statement.Condition) ? + helpers.isValidCondition(statement, allowedConditionKeys, helpers.IAM_CONDITION_OPERATORS, true, accountId) : []; if (helpers.globalPrincipal(statement.Principal)) { - if (statement.Condition && helpers.isValidCondition(statement, allowedConditionKeys, helpers.IAM_CONDITION_OPERATORS, false, accountId)) continue; - for (var a in statement.Action) { - if (globalActions.indexOf(statement.Action[a]) === -1) { - globalActions.push(statement.Action[a]); + // if (statement.Condition && helpers.isValidCondition(statement, allowedConditionKeys, helpers.IAM_CONDITION_OPERATORS, false, accountId)) continue; + if (statement.Condition && conditionalPrincipals.length) { + for (let principal of conditionalPrincipals) { + if (helpers.crossAccountPrincipal(principal, accountId)) { + crossAccountAccess = true; + break; + } + } + } else { + for (var a in statement.Action) { + if (globalActions.indexOf(statement.Action[a]) === -1) { + globalActions.push(statement.Action[a]); + } } } - } else { - let conditionalPrincipals = helpers.isValidCondition(statement, allowedConditionKeys, helpers.IAM_CONDITION_OPERATORS, true, accountId); + } + + if (helpers.crossAccountPrincipal(statement.Principal, accountId)) crossAccountAccess = true; + + if (crossAccountAccess) { if (helpers.crossAccountPrincipal(statement.Principal, accountId) || (conditionalPrincipals && conditionalPrincipals.length)) { let crossAccountPrincipals = helpers.crossAccountPrincipal(statement.Principal, accountId, true); @@ -151,15 +165,11 @@ module.exports = { if (!crossAccountPrincipals.length) continue; let crossAccount = false; - let orgAccount; for (let principal of crossAccountPrincipals) { if (config.sqs_whitelisted_aws_account_principals.includes(principal)) continue; - - if (whitelistOrganization) { - orgAccount = organizationAccounts.find(account => principal.includes(account)); - if (orgAccount) continue; - } + if (whitelistOrganization && + organizationAccounts.find(account => principal.includes(account))) continue; crossAccount = true; break; @@ -189,11 +199,9 @@ module.exports = { 'The SQS queue policy does not allow global or cross-account access.', region, queueArn); } - - cb(); - }, function(){ - rcb(); }); + + rcb(); }, function(){ callback(null, results, source); });