SLK-42343: Added condition key to be check for SQS Access Policy (aqu…

…asecurity#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
pmalkki · Jan 6, 2022 · 4a562d4 · 4a562d4
1 parent 6ca4ae1
commit 4a562d4
Showing 1 changed file with 31 additions and 23 deletions.
diff --git 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);
         });