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 5 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
1 change: 1 addition & 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 Scan Rule now supports multiple cloud providers. Implemented provider-specific metadata indicators to reduce false positives.
alessiodallapiazza marked this conversation as resolved.
Show resolved Hide resolved
- Maintenance changes.
- The Spring Actuator Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119).

Expand Down
123 changes: 110 additions & 13 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 @@ -42,17 +42,85 @@ 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);

// this class hold metadata endpoint details
private static class CloudMetadataEndpoint {
alessiodallapiazza marked this conversation as resolved.
Show resolved Hide resolved
String host;
String path;
String provider;
Map<String, String> headers;

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

// metadata endpoints to test
private static final List<CloudMetadataEndpoint> METADATA_ENDPOINTS =
Arrays.asList(
// AWS
new CloudMetadataEndpoint(
"169.254.169.254", "/latest/meta-data/", "AWS", Collections.emptyMap()),
new CloudMetadataEndpoint(
"aws.zaproxy.org", "/latest/meta-data/", "AWS", Collections.emptyMap()),
// GCP
new CloudMetadataEndpoint(
"169.254.169.254",
"/computeMetadata/v1/",
"GCP",
Map.of("Metadata-Flavor", "Google")),
new CloudMetadataEndpoint(
"metadata.google.internal",
"/computeMetadata/v1/",
"GCP",
Map.of("Metadata-Flavor", "Google")),
// OCI
new CloudMetadataEndpoint(
"169.254.169.254", "/opc/v1/instance/", "OCI", Collections.emptyMap()),
new CloudMetadataEndpoint(
"metadata.oraclecloud.com",
"/opc/v1/instance/",
"OCI",
Collections.emptyMap()),
// Alibaba Cloud
new CloudMetadataEndpoint(
"100.100.100.200",
"/latest/meta-data/",
"AlibabaCloud",
Collections.emptyMap()),
new CloudMetadataEndpoint(
"alibaba.zaproxy.org",
"/latest/meta-data/",
"AlibabaCloud",
Collections.emptyMap()),
// Azure
new CloudMetadataEndpoint(
"169.254.169.254",
"/metadata/instance",
"Azure",
Map.of("Metadata", "true")));

// metadata indicators for each cloud provider
private static final Map<String, List<String>> PROVIDER_INDICATORS =
Map.of(
"AWS",
Arrays.asList(
"ami-id", "instance-id", "local-hostname", "public-hostname"),
"GCP", Arrays.asList("project-id", "zone", "machineType", "hostname"),
"Azure", Arrays.asList("compute", "network", "osType", "vmSize"),
"AlibabaCloud",
Arrays.asList("image-id", "instance-id", "hostname", "region-id"),
"OCI", Arrays.asList("oci", "instance", "availabilityDomain", "region"));

@Override
public int getId() {
return PLUGIN_ID;
Expand Down Expand Up @@ -95,28 +163,57 @@ 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) {
for (CloudMetadataEndpoint endpoint : METADATA_ENDPOINTS) {
HttpMessage newRequest = getNewMsg();
try {
newRequest.getRequestHeader().getURI().setPath(METADATA_PATH);
newRequest.setUserObject(Collections.singletonMap("host", host));
// set the request path
alessiodallapiazza marked this conversation as resolved.
Show resolved Hide resolved
newRequest.getRequestHeader().getURI().setPath(endpoint.path);
// set the Host header
newRequest.setUserObject(Collections.singletonMap("host", endpoint.host));
// set additional headers if required
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) {
this.createAlert(newRequest, host).raise();
return;
String responseBody = newRequest.getResponseBody().toString();
if (containsMetadataIndicators(responseBody, endpoint.provider)) {
this.createAlert(newRequest, endpoint.host).raise();
return;
}
}
} catch (Exception e) {
LOGGER.warn("Error sending URL {}", newRequest.getRequestHeader().getURI(), e);
LOGGER.warn("Error sending request to {}: {}", endpoint.host, e.getMessage(), e);
}
}
}

/**
* Checks if the response body contains metadata indicators specific to the cloud provider.
*
* @param responseBody the response body to check
* @param provider the cloud provider
* @return {@code true} if cloud metadata indicators are found; {@code false} otherwise
*/
private boolean containsMetadataIndicators(String responseBody, String provider) {
alessiodallapiazza marked this conversation as resolved.
Show resolved Hide resolved
List<String> indicators = PROVIDER_INDICATORS.get(provider);
if (indicators == null) {
return false;
}
for (String indicator : indicators) {
if (responseBody.contains(indicator)) {
return true;
}
}
return false;
}

@Override
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