Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ascanrules: fix false positive in cloud metadata #5729

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

ascanrules: fix false positive in cloud metadata #5729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0ccf4f3
add a method to check if the body contains metadata information
alessiodallapiazza Sep 17, 2024
3ef78f4
fix test and createAlert
alessiodallapiazza Sep 17, 2024
bfe147a
apply spotlessApply
alessiodallapiazza Sep 17, 2024
1b2cc0a
add more tests and more checks for cloud-metadata
alessiodallapiazza Sep 17, 2024
73f2c1e
update changelog
alessiodallapiazza Sep 17, 2024
c5d946b
update changelog
alessiodallapiazza Sep 17, 2024
ab7a9bf
apply review
alessiodallapiazza Sep 17, 2024
80afb76
use List.of
alessiodallapiazza Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions addOns/ascanrules/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased
### Changed
- Cloud Metadata rule is improved to support GCP, Azure and OCI.
- Maintenance changes.
- The Spring Actuator Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119).

Expand All @@ -19,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- False positives in the Path Traversal rule.
- Alert text for various rules has been updated to more consistently use periods and spaces in a uniform manner.
- False Positives in the Remote File Inclusion rule (Issue 8561).
- False Positives in Cloud Metadata rule.

## [66] - 2024-05-07
### Changed
Expand Down
119 changes: 101 additions & 18 deletions .../ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CloudMetadataScanRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
*/
package org.zaproxy.zap.extension.ascanrules;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.parosproxy.paros.Constant;
Expand All @@ -42,17 +42,91 @@ public class CloudMetadataScanRule extends AbstractHostPlugin implements CommonA
private static final String MESSAGE_PREFIX = "ascanrules.cloudmetadata.";

private static final int PLUGIN_ID = 90034;
private static final String METADATA_PATH = "/latest/meta-data/";
private static final List<String> METADATA_HOSTS =
Arrays.asList(
"169.254.169.254", "aws.zaproxy.org", "100.100.100.200", "alibaba.zaproxy.org");

private static final Logger LOGGER = LogManager.getLogger(CloudMetadataScanRule.class);
private static final Map<String, String> ALERT_TAGS =
CommonAlertTag.toMap(
CommonAlertTag.OWASP_2021_A05_SEC_MISCONFIG,
CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG);

private enum CloudProvider {
AWS(
List.of(
new Endpoint(
"169.254.169.254", "/latest/meta-data/", Collections.emptyMap()),
new Endpoint(
"aws.zaproxy.org", "/latest/meta-data/", Collections.emptyMap())),
Set.of("ami-id", "instance-id", "local-hostname", "public-hostname")),
GCP(
List.of(
new Endpoint(
"169.254.169.254",
"/computeMetadata/v1/",
Map.of("Metadata-Flavor", "Google")),
new Endpoint(
"metadata.google.internal",
"/computeMetadata/v1/",
Map.of("Metadata-Flavor", "Google"))),
Set.of("project-id", "zone", "machineType", "hostname")),
OCI(
List.of(
new Endpoint(
"169.254.169.254", "/opc/v1/instance/", Collections.emptyMap()),
new Endpoint(
"metadata.oraclecloud.com",
"/opc/v1/instance/",
Collections.emptyMap())),
Set.of("oci", "instance", "availabilityDomain", "region")),
AlibabaCloud(
List.of(
new Endpoint(
"100.100.100.200", "/latest/meta-data/", Collections.emptyMap()),
new Endpoint(
"alibaba.zaproxy.org",
"/latest/meta-data/",
Collections.emptyMap())),
Set.of("image-id", "instance-id", "hostname", "region-id")),
Azure(
List.of(
new Endpoint(
"169.254.169.254",
"/metadata/instance",
Map.of("Metadata", "true"))),
Set.of("compute", "network", "osType", "vmSize"));

private final List<Endpoint> endpoints;
private final Set<String> indicators;

CloudProvider(List<Endpoint> endpoints, Set<String> indicators) {
this.endpoints = endpoints;
this.indicators = indicators;
}

public List<Endpoint> getEndpoints() {
return endpoints;
}

public boolean containsMetadataIndicators(String responseBody) {
for (String indicator : indicators) {
if (responseBody.contains(indicator)) {
return true;
}
}
return false;
}

private static class Endpoint {
String host;
String path;
Map<String, String> headers;

Endpoint(String host, String path, Map<String, String> headers) {
this.host = host;
this.path = path;
this.headers = headers;
}
}
}

@Override
public int getId() {
return PLUGIN_ID;
Expand Down Expand Up @@ -95,26 +169,35 @@ public Map<String, String> getAlertTags() {

public AlertBuilder createAlert(HttpMessage newRequest, String host) {
return newAlert()
.setConfidence(Alert.CONFIDENCE_LOW)
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setAttack(host)
.setOtherInfo(Constant.messages.getString(MESSAGE_PREFIX + "otherinfo"))
.setMessage(newRequest);
}

@Override
public void scan() {
HttpMessage newRequest = getNewMsg();
for (String host : METADATA_HOSTS) {
try {
newRequest.getRequestHeader().getURI().setPath(METADATA_PATH);
newRequest.setUserObject(Collections.singletonMap("host", host));
sendAndReceive(newRequest, false);
if (isSuccess(newRequest) && newRequest.getResponseBody().length() > 0) {
this.createAlert(newRequest, host).raise();
return;
for (CloudProvider provider : CloudProvider.values()) {
for (CloudProvider.Endpoint endpoint : provider.getEndpoints()) {
HttpMessage newRequest = getNewMsg();
try {
newRequest.getRequestHeader().getURI().setPath(endpoint.path);
newRequest.setUserObject(Collections.singletonMap("host", endpoint.host));
for (Map.Entry<String, String> header : endpoint.headers.entrySet()) {
newRequest.getRequestHeader().setHeader(header.getKey(), header.getValue());
}
sendAndReceive(newRequest, false);
if (isSuccess(newRequest) && newRequest.getResponseBody().length() > 0) {
String responseBody = newRequest.getResponseBody().toString();
if (provider.containsMetadataIndicators(responseBody)) {
this.createAlert(newRequest, endpoint.host).raise();
return;
}
}
} catch (Exception e) {
LOGGER.warn(
"Error sending request to {}: {}", endpoint.host, e.getMessage(), e);
}
} catch (Exception e) {
LOGGER.warn("Error sending URL {}", newRequest.getRequestHeader().getURI(), e);
}
}
}
Expand Down
73 changes: 69 additions & 4 deletions ...les/src/test/java/org/zaproxy/zap/extension/ascanrules/CloudMetadataScanRuleUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ void shouldNotAlertIfResponseIsNot200Ok() throws Exception {
strings = {
"169.254.169.254",
"aws.zaproxy.org",
"100.100.100.200",
"alibaba.zaproxy.org"
})
void shouldAlertIfResponseIs200Ok(String host) throws Exception {
void shouldAlertIfResponseIs200OkAWS(String host) throws Exception {
// Given
String path = "/latest/meta-data/";
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html
Expand All @@ -87,7 +85,74 @@ void shouldAlertIfResponseIs200Ok(String host) throws Exception {
assertThat(alertsRaised, hasSize(1));
Alert alert = alertsRaised.get(0);
assertEquals(Alert.RISK_HIGH, alert.getRisk());
assertEquals(Alert.CONFIDENCE_LOW, alert.getConfidence());
assertEquals(Alert.CONFIDENCE_MEDIUM, alert.getConfidence());
assertEquals(host, alert.getAttack());
}

@ParameterizedTest
@ValueSource(
strings = {
"100.100.100.200",
"alibaba.zaproxy.org",
})
void shouldAlertIfResponseIs200OkAlibabaCloud(String host) throws Exception {
// Given
String path = "/latest/meta-data/";
String body = "image-id\ninstance-id";
this.nano.addHandler(createHandler(path, Response.Status.OK, body, host));
HttpMessage msg = this.getHttpMessage(path);
rule.init(msg, this.parent);
// When
rule.scan();
// Then
assertThat(alertsRaised, hasSize(1));
Alert alert = alertsRaised.get(0);
assertEquals(Alert.RISK_HIGH, alert.getRisk());
assertEquals(Alert.CONFIDENCE_MEDIUM, alert.getConfidence());
assertEquals(host, alert.getAttack());
}

@ParameterizedTest
@ValueSource(
strings = {
"169.254.169.254",
})
void shouldAlertIfResponseIs200OkGCP(String host) throws Exception {
// Given
String path = "/computeMetadata/v1/";
String body = "project-id";
this.nano.addHandler(createHandler(path, Response.Status.OK, body, host));
HttpMessage msg = this.getHttpMessage(path);
rule.init(msg, this.parent);
// When
rule.scan();
// Then
assertThat(alertsRaised, hasSize(1));
Alert alert = alertsRaised.get(0);
assertEquals(Alert.RISK_HIGH, alert.getRisk());
assertEquals(Alert.CONFIDENCE_MEDIUM, alert.getConfidence());
assertEquals(host, alert.getAttack());
}

@ParameterizedTest
@ValueSource(
strings = {
"169.254.169.254",
})
void shouldAlertIfResponseIs200OkAzure(String host) throws Exception {
// Given
String path = "/metadata/instance";
String body = "osType";
this.nano.addHandler(createHandler(path, Response.Status.OK, body, host));
HttpMessage msg = this.getHttpMessage(path);
rule.init(msg, this.parent);
// When
rule.scan();
// Then
assertThat(alertsRaised, hasSize(1));
Alert alert = alertsRaised.get(0);
assertEquals(Alert.RISK_HIGH, alert.getRisk());
assertEquals(Alert.CONFIDENCE_MEDIUM, alert.getConfidence());
assertEquals(host, alert.getAttack());
}

Expand Down