Skip to content

Commit 5f00640

Browse files
joke1196sonartech
joke1196
authored and
sonartech
committed
SONARPY-1012: Rule S6249: Authorizing HTTP communications with S3 buckets is security-sensitive. (#402)
GitOrigin-RevId: 27d012893267f2ecfb7b32380e70adad1e2aab2e
1 parent 223bac8 commit 5f00640

File tree

7 files changed

+457
-0
lines changed

7 files changed

+457
-0
lines changed

‎python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.sonar.python.checks.cdk.ResourceAccessPolicyCheck;
3030
import org.sonar.python.checks.cdk.S3BucketBlockPublicAccessCheck;
3131
import org.sonar.python.checks.cdk.S3BucketGrantedAccessCheck;
32+
import org.sonar.python.checks.cdk.S3BucketHTTPCommunicationCheck;
3233
import org.sonar.python.checks.cdk.S3BucketServerEncryptionCheck;
3334
import org.sonar.python.checks.cdk.S3BucketVersioningCheck;
3435
import org.sonar.python.checks.cdk.UnencryptedEbsVolumeCheck;
@@ -343,6 +344,7 @@ public Stream<Class<?>> getChecks() {
343344
RobustCipherAlgorithmCheck.class,
344345
S3BucketBlockPublicAccessCheck.class,
345346
S3BucketGrantedAccessCheck.class,
347+
S3BucketHTTPCommunicationCheck.class,
346348
S3BucketServerEncryptionCheck.class,
347349
S3BucketVersioningCheck.class,
348350
SameBranchCheck.class,

‎python-checks/src/main/java/org/sonar/python/checks/cdk/S3BucketHTTPCommunicationCheck.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks.cdk;
18+
19+
import java.util.Optional;
20+
import java.util.function.BiConsumer;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
23+
import org.sonar.plugins.python.api.symbols.Usage;
24+
import org.sonar.plugins.python.api.tree.AssignmentStatement;
25+
import org.sonar.plugins.python.api.tree.CallExpression;
26+
import org.sonar.plugins.python.api.tree.Expression;
27+
import org.sonar.plugins.python.api.tree.ListLiteral;
28+
import org.sonar.plugins.python.api.tree.Name;
29+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
30+
import org.sonar.plugins.python.api.tree.RegularArgument;
31+
import org.sonar.plugins.python.api.tree.Tree;
32+
import org.sonar.python.tree.TreeUtils;
33+
34+
@Rule(key = "S6249")
35+
public class S3BucketHTTPCommunicationCheck extends AbstractS3BucketCheck {
36+
37+
private static final String MESSAGE_NO_POLICY = "No bucket policy enforces HTTPS-only access to this bucket. Make sure it is safe here.";
38+
private static final String MESSAGE_HTTP_ALLOWED = "Make sure authorizing HTTP requests is safe here.";
39+
private static final String POLICY_STATEMENT_FQN = "aws_cdk.aws_iam.PolicyStatement";
40+
41+
@Override
42+
BiConsumer<SubscriptionContext, CallExpression> visitBucketConstructor() {
43+
return S3BucketHTTPCommunicationCheck::checkBucketConstructor;
44+
}
45+
46+
private static void checkBucketConstructor(SubscriptionContext ctx, CallExpression bucketConstructorCall) {
47+
48+
var enforceSslArg = CdkUtils.getArgument(ctx, bucketConstructorCall, "enforce_ssl");
49+
50+
if (enforceSslArg.isEmpty()) {
51+
// No enforce_ssl parameter specified
52+
// check if this bucket has add_to_resource_policy calls and a correct policy statement
53+
getBucketPolicy(bucketConstructorCall)
54+
.flatMap(S3BucketHTTPCommunicationCheck::getPolicyStatementConstructor)
55+
.ifPresentOrElse(policyStatementConstructorCall -> visitPolicyStatement(ctx, policyStatementConstructorCall),
56+
() -> ctx.addIssue(bucketConstructorCall.callee(), MESSAGE_NO_POLICY));
57+
} else {
58+
enforceSslArg.get().addIssueIf(CdkPredicate.isFalse(), MESSAGE_HTTP_ALLOWED, bucketConstructorCall);
59+
}
60+
}
61+
62+
private static Optional<CallExpression> getBucketPolicy(CallExpression bucketConstructor) {
63+
return Optional.ofNullable(TreeUtils.firstAncestorOfKind(bucketConstructor, Tree.Kind.ASSIGNMENT_STMT))
64+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(AssignmentStatement.class))
65+
.flatMap(S3BucketHTTPCommunicationCheck::getFirstAndOnlyAssignedVariable)
66+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
67+
.flatMap(S3BucketHTTPCommunicationCheck::getAddToResourcePolicyCall);
68+
}
69+
70+
private static Optional<Expression> getFirstAndOnlyAssignedVariable(AssignmentStatement assignmentStatement) {
71+
return assignmentStatement.lhsExpressions().stream()
72+
.filter(lhs -> lhs.expressions().size() == 1)
73+
.map(lhs -> lhs.expressions().get(0))
74+
.findFirst();
75+
}
76+
77+
private static Optional<CallExpression> getAddToResourcePolicyCall(Name variableName) {
78+
var symbol = TreeUtils.getSymbolFromTree(variableName);
79+
if (symbol.isEmpty()) {
80+
return Optional.empty();
81+
}
82+
83+
// Check if the usage is part of a qualified expression like "bucket.add_to_resource_policy"
84+
return symbol.get().usages().stream()
85+
.filter(usage -> usage.kind() != Usage.Kind.ASSIGNMENT_LHS)
86+
.map(usage -> usage.tree().parent())
87+
.flatMap(TreeUtils.toStreamInstanceOfMapper(QualifiedExpression.class))
88+
.filter(qualifiedExpr -> CdkPredicate.isFqn("add_to_resource_policy").test(qualifiedExpr.name()))
89+
.map(QualifiedExpression::parent)
90+
.flatMap(TreeUtils.toStreamInstanceOfMapper(CallExpression.class))
91+
.findFirst();
92+
}
93+
94+
private static Optional<CallExpression> getPolicyStatementConstructor(CallExpression addResourceToPolicyCall) {
95+
if (!addResourceToPolicyCall.arguments().isEmpty()) {
96+
return Optional.of(addResourceToPolicyCall.arguments().get(0))
97+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(RegularArgument.class))
98+
.map(RegularArgument::expression)
99+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(CallExpression.class))
100+
.filter(CdkPredicate.isFqn(POLICY_STATEMENT_FQN)::test);
101+
}
102+
return Optional.empty();
103+
}
104+
105+
private static void visitPolicyStatement(SubscriptionContext ctx, CallExpression policyStatementConstructorCall) {
106+
if (!isPolicyCompliantForHttpsDeny(ctx, policyStatementConstructorCall)) {
107+
ctx.addIssue(policyStatementConstructorCall.callee(), MESSAGE_HTTP_ALLOWED);
108+
}
109+
}
110+
111+
private static boolean isPolicyCompliantForHttpsDeny(SubscriptionContext ctx, CallExpression policyStatementConstructorCall) {
112+
// A compliant policy should have at least the following keyword arguments:
113+
// 1. effect=iam.Effect.DENY
114+
// 2. resources=["*"] (wildcard)
115+
// 3. actions=["s3:*"] (wildcard)
116+
// 4. principals=["*"] (wildcard)
117+
// 5. conditions=["SecureTransport:False"]
118+
119+
boolean hasDenyEffect = hasArgumentWithFqn(ctx, policyStatementConstructorCall, "effect", "aws_cdk.aws_iam.Effect.DENY");
120+
boolean hasWildcardResources = argumentListContainsExpectedValue(ctx, policyStatementConstructorCall, "resources", "*");
121+
boolean hasWildcardActions = argumentListContainsExpectedValue(ctx, policyStatementConstructorCall, "actions", "s3:*");
122+
boolean hasWildcardPrincipals = argumentListContainsExpectedValue(ctx, policyStatementConstructorCall, "principals", "*");
123+
boolean hasSecureTransportCondition = argumentListContainsExpectedValue(ctx, policyStatementConstructorCall, "conditions", "SecureTransport:False");
124+
125+
return hasDenyEffect && hasWildcardResources && hasWildcardActions && hasWildcardPrincipals && hasSecureTransportCondition;
126+
}
127+
128+
private static boolean hasArgumentWithFqn(SubscriptionContext ctx, CallExpression call, String argName, String expectedFqn) {
129+
return CdkUtils.getArgument(ctx, call, argName)
130+
.map(flow -> flow.hasExpression(CdkPredicate.isFqn(expectedFqn)))
131+
.orElse(false);
132+
}
133+
134+
private static boolean argumentListContainsExpectedValue(SubscriptionContext ctx, CallExpression call, String argName, String expectedValue) {
135+
return CdkUtils.getArgument(ctx, call, argName)
136+
.flatMap(flow -> flow.getExpression(e -> e.is(Tree.Kind.LIST_LITERAL)))
137+
.map(ListLiteral.class::cast)
138+
.map(list -> list.elements().expressions().stream()
139+
.anyMatch(expr -> CdkUtils.getString(expr).filter(expectedValue::equals).isPresent()))
140+
.orElse(false);
141+
}
142+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6249.html‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<p>By default, S3 buckets can be accessed through HTTP and HTTPs protocols.</p>
2+
<p>As HTTP is a clear-text protocol, it lacks the encryption of transported data, as well as the capability to build an authenticated connection. It
3+
means that a malicious actor who is able to intercept traffic from the network can read, modify or corrupt the transported content.</p>
4+
<h2>Ask Yourself Whether</h2>
5+
<ul>
6+
<li> The S3 bucket stores sensitive information. </li>
7+
<li> The infrastructure has to comply with AWS Foundational Security Best Practices standard. </li>
8+
</ul>
9+
<p>There is a risk if you answered yes to any of those questions.</p>
10+
<h2>Recommended Secure Coding Practices</h2>
11+
<p>It’s recommended to deny all HTTP requests:</p>
12+
<ul>
13+
<li> for all objects (<code>*</code>) of the bucket </li>
14+
<li> for all principals (<code>*</code>) </li>
15+
<li> for all actions (<code>*</code>) </li>
16+
</ul>
17+
<h2>Sensitive Code Example</h2>
18+
<p>No secure policy is attached to this bucket:</p>
19+
<pre>
20+
import aws_cdk.aws_s3 as s3
21+
import aws_cdk.aws_iam as iam
22+
23+
bucket = s3.Bucket(self, "bucket") # Sensitive
24+
</pre>
25+
<p>A policy is defined but forces only HTTPs communication for some users, some objects of the bucket and for some actions:</p>
26+
<pre>
27+
bucket = s3.Bucket(self, "bucket")
28+
bucket.add_to_resource_policy(iam.PolicyStatement( # Sensitive
29+
effect=iam.Effect.DENY,
30+
resources=[bucket.bucket_arn],
31+
actions=["s3:SomeAction"],
32+
principals=[roles],
33+
conditions=[{"Bool": {"aws:SecureTransport": False}}]
34+
)
35+
)
36+
</pre>
37+
<h2>Compliant Solution</h2>
38+
<p>A bucket policy that complies with s3-bucket-ssl-requests-only rule should be used. To adhere to it, the bucket policies need to explicitly deny
39+
access to HTTP requests.</p>
40+
<p>A secure policy that enforces SSL on requests (default: False):</p>
41+
<pre>
42+
bucket = S3.Bucket(self,
43+
"bucket",
44+
enforce_ssl=True
45+
)
46+
</pre>
47+
<p>A secure policy that denies all HTTP requests is used:</p>
48+
<pre>
49+
bucket = s3.Bucket(self, "bucket")
50+
51+
result = bucket.add_to_resource_policy(iam.PolicyStatement(
52+
effect=iam.Effect.DENY,
53+
resources=["*"],
54+
actions=["s3:*"],
55+
principals=["*"],
56+
conditions=["SecureTransport:False"]
57+
)
58+
)
59+
</pre>
60+
<h2>See</h2>
61+
<ul>
62+
<li> <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/security-best-practices.html#transit">AWS documentation</a> - Enforce encryption
63+
of data in transit </li>
64+
<li> <a href="https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-s3-5">AWS Foundational Security
65+
Best Practices controls</a> - S3 buckets should require requests to use Secure Socket Layer </li>
66+
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/319">CWE-319 - Cleartext Transmission of Sensitive Information</a> </li>
67+
<li> <a href="https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-s3-5">AWS Foundational Security
68+
Best Practices controls</a> - S3 buckets should require requests to use Secure Socket Layer </li>
69+
</ul>
70+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6249.json‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"title": "Authorizing HTTP communications with S3 buckets is security-sensitive",
3+
"type": "SECURITY_HOTSPOT",
4+
"code": {
5+
"impacts": {
6+
"SECURITY": "HIGH"
7+
},
8+
"attribute": "COMPLETE"
9+
},
10+
"status": "ready",
11+
"remediation": {
12+
"func": "Constant\/Issue",
13+
"constantCost": "10min"
14+
},
15+
"tags": [
16+
"aws",
17+
"cwe"
18+
],
19+
"defaultSeverity": "Critical",
20+
"ruleSpecification": "RSPEC-6249",
21+
"sqKey": "S6249",
22+
"scope": "Main",
23+
"securityStandards": {
24+
"CWE": [
25+
319
26+
],
27+
"PCI DSS 3.2": [
28+
"4.1",
29+
"6.5.4"
30+
],
31+
"PCI DSS 4.0": [
32+
"4.2.1",
33+
"6.2.4"
34+
]
35+
}
36+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@
168168
"S6002",
169169
"S6019",
170170
"S6035",
171+
"S6249",
171172
"S6252",
172173
"S6265",
173174
"S6270",

‎python-checks/src/test/java/org/sonar/python/checks/cdk/S3BucketHTTPCommunicationCheckTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks.cdk;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class S3BucketHTTPCommunicationCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/cdk/s3BucketHTTPCommunication.py", new S3BucketHTTPCommunicationCheck());
27+
}
28+
29+
}
30+

0 commit comments

Comments
 (0)