Skip to content

Commit 518f493

Browse files
kingthorinkingthorin
committed
Reduce use of error level logging in pscan rules, tech detection, &
retire - Add change note. - Update logging in scan rules. Signed-off-by: kingthorin <kingthorin@users.noreply.github.com> # Conflicts: # addOns/wappalyzer/CHANGELOG.md
1 parent 7bed6b9 commit 518f493

File tree

17 files changed

+642
-665
lines changed

17 files changed

+642
-665
lines changed

‎addOns/pscanrules/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
55

66
## Unreleased
7-
7+
### Changed
8+
- Reduced usage of error level logging.
89

910
## [66] - 2025-07-25
1011
### Added

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/CrossDomainMisconfigurationScanRule.java‎

Lines changed: 53 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -85,66 +85,60 @@ public String getName() {
8585
@Override
8686
public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
8787

88-
try {
88+
LOGGER.debug(
89+
"Checking message {} for Cross-Domain misconfigurations",
90+
msg.getRequestHeader().getURI());
91+
92+
String corsAllowOriginValue =
93+
msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN);
94+
// String corsAllowHeadersValue =
95+
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_HEADERS);
96+
// String corsAllowMethodsValue =
97+
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_METHODS);
98+
// String corsExposeHeadersValue =
99+
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_EXPOSE_HEADERS);
100+
101+
if (corsAllowOriginValue != null && corsAllowOriginValue.equals("*")) {
89102
LOGGER.debug(
90-
"Checking message {} for Cross-Domain misconfigurations",
91-
msg.getRequestHeader().getURI());
92-
93-
String corsAllowOriginValue =
94-
msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN);
95-
// String corsAllowHeadersValue =
96-
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_HEADERS);
97-
// String corsAllowMethodsValue =
98-
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_ALLOW_METHODS);
99-
// String corsExposeHeadersValue =
100-
// msg.getResponseHeader().getHeader(HttpHeader.ACCESS_CONTROL_EXPOSE_HEADERS);
101-
102-
if (corsAllowOriginValue != null && corsAllowOriginValue.equals("*")) {
103-
LOGGER.debug(
104-
"Raising a Medium risk Cross Domain alert on {}: {}",
105-
HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN,
106-
corsAllowOriginValue);
107-
// Its a Medium, rather than a High (as originally thought), for the following
108-
// reasons:
109-
// Assumption: if an API is accessible in an unauthenticated manner, it doesn't need
110-
// to be protected
111-
// (if it should be protected, its a Missing Function Level Access Control issue,
112-
// not a Cross Domain Misconfiguration)
113-
//
114-
// Case 1) Request sent using XHR
115-
// - cookies will not be sent with the request at all unless withCredentials = true
116-
// on the XHR request;
117-
// - If a cookie was sent with the request, the browser will not give access to the
118-
// response body via JavaScript unless the response headers say
119-
// "Access-Control-Allow-Credentials: true"
120-
// - If "Access-Control-Allow-Credentials: true" and "Access-Control-Allow-Origin:
121-
// *" in the response, the browser will not give access to the response body.
122-
// (this is an edge case, but is actually really important, because it blocks all
123-
// the useful attacks, and is well supported by modern browsers)
124-
// Case 2) Request sent using HTML Form POST with an iframe, for instance, and
125-
// attempting to access the iframe body (ie, the Cross Domain response) using
126-
// JavaScript
127-
// - the cookie will be sent by the web browser (possibly leading to CSRF, but with
128-
// no impact from the point of view of the Same Origin Policy / Cross Domain
129-
// Misconfiguration
130-
// - the HTML response is not accessible in JavaScript, regardless of the CORS
131-
// headers sent in the response (in all my trials, at least)
132-
// (this is even more restrictive than the equivalent request sent by XHR)
133-
134-
// The CORS misconfig could still allow an attacker to access the data returned from
135-
// an unauthenticated API, which is protected by some other form of security, such
136-
// as IP address white-listing, for instance.
137-
138-
buildAlert(
139-
extractEvidence(
140-
msg.getResponseHeader().toString(),
141-
HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN))
142-
.raise();
143-
}
144-
145-
} catch (Exception e) {
146-
LOGGER.error(
147-
"An error occurred trying to passively scan a message for Cross Domain Misconfigurations");
103+
"Raising a Medium risk Cross Domain alert on {}: {}",
104+
HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN,
105+
corsAllowOriginValue);
106+
// Its a Medium, rather than a High (as originally thought), for the following
107+
// reasons:
108+
// Assumption: if an API is accessible in an unauthenticated manner, it doesn't need
109+
// to be protected
110+
// (if it should be protected, its a Missing Function Level Access Control issue,
111+
// not a Cross Domain Misconfiguration)
112+
//
113+
// Case 1) Request sent using XHR
114+
// - cookies will not be sent with the request at all unless withCredentials = true
115+
// on the XHR request;
116+
// - If a cookie was sent with the request, the browser will not give access to the
117+
// response body via JavaScript unless the response headers say
118+
// "Access-Control-Allow-Credentials: true"
119+
// - If "Access-Control-Allow-Credentials: true" and "Access-Control-Allow-Origin:
120+
// *" in the response, the browser will not give access to the response body.
121+
// (this is an edge case, but is actually really important, because it blocks all
122+
// the useful attacks, and is well supported by modern browsers)
123+
// Case 2) Request sent using HTML Form POST with an iframe, for instance, and
124+
// attempting to access the iframe body (ie, the Cross Domain response) using
125+
// JavaScript
126+
// - the cookie will be sent by the web browser (possibly leading to CSRF, but with
127+
// no impact from the point of view of the Same Origin Policy / Cross Domain
128+
// Misconfiguration
129+
// - the HTML response is not accessible in JavaScript, regardless of the CORS
130+
// headers sent in the response (in all my trials, at least)
131+
// (this is even more restrictive than the equivalent request sent by XHR)
132+
133+
// The CORS misconfig could still allow an attacker to access the data returned from
134+
// an unauthenticated API, which is protected by some other form of security, such
135+
// as IP address white-listing, for instance.
136+
137+
buildAlert(
138+
extractEvidence(
139+
msg.getResponseHeader().toString(),
140+
HttpHeader.ACCESS_CONTROL_ALLOW_ORIGIN))
141+
.raise();
148142
}
149143
}
150144

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InformationDisclosureDebugErrorsScanRule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ private static List<String> loadFile(Path path) {
114114
BufferedReader reader = null;
115115
File f = path.toFile();
116116
if (!f.exists()) {
117-
LOGGER.error("No such file: {}", f.getAbsolutePath());
117+
LOGGER.warn("No such file: {}", f.getAbsolutePath());
118118
return strings;
119119
}
120120
try {

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InformationDisclosureInUrlScanRule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ private static List<String> loadFile(String file) {
132132
List<String> strings = new ArrayList<>();
133133
File f = new File(Constant.getZapHome() + File.separator + file);
134134
if (!f.exists()) {
135-
LOGGER.error("No such file: {}", f.getAbsolutePath());
135+
LOGGER.warn("No such file: {}", f.getAbsolutePath());
136136
return strings;
137137
}
138138

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InformationDisclosureReferrerScanRule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ private static List<String> loadFile(String file) {
184184
List<String> strings = new ArrayList<>();
185185
File f = new File(Constant.getZapHome() + File.separator + file);
186186
if (!f.exists()) {
187-
LOGGER.error("No such file: {}", f.getAbsolutePath());
187+
LOGGER.warn("No such file: {}", f.getAbsolutePath());
188188
return strings;
189189
}
190190

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public void scanHttpRequestSend(HttpMessage msg, int id) {
124124
alertRisk = Alert.RISK_HIGH;
125125
}
126126
} catch (IllegalArgumentException e) {
127-
LOGGER.error(
127+
LOGGER.warn(
128128
"Invalid Base64 value for {} Authentication: {}",
129129
authMechanism,
130130
authValues[1]);

‎addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/RetrievedFromCacheScanRule.java‎

Lines changed: 69 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -57,88 +57,82 @@ public class RetrievedFromCacheScanRule extends PluginPassiveScanner
5757
@Override
5858
public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
5959

60-
try {
61-
LOGGER.debug(
62-
"Checking URL {} to see if was served from a shared cache",
63-
msg.getRequestHeader().getURI());
64-
65-
// X-Cache: HIT
66-
// X-Cache: HIT from cache.kolich.local <-- was the data actually served from the
67-
// cache (subject to no-cache, expiry, etc.)?
68-
// (if X-Cache: HIT, it implies X-Cache-Lookup: HIT)
69-
// (and if X-Cache-Lookup: MISS, it implies X-Cache: MISS)
70-
// X-Cache-Lookup: HIT from cache.kolich.local:80 <-- was the data *available* in the
71-
// cache? (not whether it was actually served)
72-
73-
// X-Cache: MISS
74-
// X-Cache: MISS from cache.kolich.local
75-
// X-Cache-Lookup: MISS from cache.kolich.local:80
76-
77-
// X-Cache HIT from proxy.domain.tld, MISS from proxy.local
78-
// X-Cache-Lookup HIT from proxy.domain.tld:3128, MISS from proxy.local:3128
79-
80-
List<String> xcacheHeaders = msg.getResponseHeader().getHeaderValues("X-Cache");
81-
if (!xcacheHeaders.isEmpty()) {
82-
for (String xcacheHeader : xcacheHeaders) {
83-
for (String proxyServerDetails : xcacheHeader.split(",")) {
84-
// strip off any leading space for the second and subsequent proxies
85-
if (proxyServerDetails.startsWith(" "))
86-
proxyServerDetails = proxyServerDetails.substring(1);
87-
LOGGER.trace("Proxy HIT/MISS details [{}]", proxyServerDetails);
88-
String[] proxyServerDetailsArray = proxyServerDetails.split(" ", 3);
89-
if (proxyServerDetailsArray.length >= 1) {
90-
String hitormiss =
91-
proxyServerDetailsArray[0].toUpperCase(); // HIT or MISS
92-
if (hitormiss.equals("HIT")) {
93-
// the response was served from cache, so raise it..
94-
String evidence = proxyServerDetails;
95-
LOGGER.debug(
96-
"{} was served from a cache, due to presence of a 'HIT' in the 'X-Cache' response header",
97-
msg.getRequestHeader().getURI());
98-
// could be from HTTP/1.0 or HTTP/1.1. We don't know which.
99-
buildAlert(evidence, false).raise();
100-
return;
101-
}
60+
LOGGER.debug(
61+
"Checking URL {} to see if was served from a shared cache",
62+
msg.getRequestHeader().getURI());
63+
64+
// X-Cache: HIT
65+
// X-Cache: HIT from cache.kolich.local <-- was the data actually served from the
66+
// cache (subject to no-cache, expiry, etc.)?
67+
// (if X-Cache: HIT, it implies X-Cache-Lookup: HIT)
68+
// (and if X-Cache-Lookup: MISS, it implies X-Cache: MISS)
69+
// X-Cache-Lookup: HIT from cache.kolich.local:80 <-- was the data *available* in the
70+
// cache? (not whether it was actually served)
71+
72+
// X-Cache: MISS
73+
// X-Cache: MISS from cache.kolich.local
74+
// X-Cache-Lookup: MISS from cache.kolich.local:80
75+
76+
// X-Cache HIT from proxy.domain.tld, MISS from proxy.local
77+
// X-Cache-Lookup HIT from proxy.domain.tld:3128, MISS from proxy.local:3128
78+
79+
List<String> xcacheHeaders = msg.getResponseHeader().getHeaderValues("X-Cache");
80+
if (!xcacheHeaders.isEmpty()) {
81+
for (String xcacheHeader : xcacheHeaders) {
82+
for (String proxyServerDetails : xcacheHeader.split(",")) {
83+
// strip off any leading space for the second and subsequent proxies
84+
if (proxyServerDetails.startsWith(" "))
85+
proxyServerDetails = proxyServerDetails.substring(1);
86+
LOGGER.trace("Proxy HIT/MISS details [{}]", proxyServerDetails);
87+
String[] proxyServerDetailsArray = proxyServerDetails.split(" ", 3);
88+
if (proxyServerDetailsArray.length >= 1) {
89+
String hitormiss = proxyServerDetailsArray[0].toUpperCase(); // HIT or MISS
90+
if (hitormiss.equals("HIT")) {
91+
// the response was served from cache, so raise it..
92+
String evidence = proxyServerDetails;
93+
LOGGER.debug(
94+
"{} was served from a cache, due to presence of a 'HIT' in the 'X-Cache' response header",
95+
msg.getRequestHeader().getURI());
96+
// could be from HTTP/1.0 or HTTP/1.1. We don't know which.
97+
buildAlert(evidence, false).raise();
98+
return;
10299
}
103100
}
104101
}
105102
}
103+
}
106104

107-
// The "Age" header (defined in RFC 7234) conveys the sender's estimate of the amount of
108-
// time since the response (or its revalidation) was generated at the origin server.
109-
// An HTTP/1.1 server that includes a cache MUST include an Age header field in every
110-
// response generated from its own cache.
111-
// i.e.: a valid "Age" header implies that the response was served from a cache
112-
// lets validate that it is actually a non-negative decimal integer, as mandated by RFC
113-
// 7234, however.
114-
// if there are multiple "Age" headers, just look for one valid value in the multiple
115-
// "Age" headers.. Not sure if this case is strictly valid with the spec, however.
116-
// Note: HTTP/1.0 caches do not implement "Age", so the absence of an "Age" header does
117-
// *not* imply that the response was served from the origin server, rather than a
118-
// cache..
119-
List<String> ageHeaders = msg.getResponseHeader().getHeaderValues("Age");
120-
if (!ageHeaders.isEmpty()) {
121-
for (String ageHeader : ageHeaders) {
122-
LOGGER.trace("Validating Age header value [{}]", ageHeader);
123-
Long ageAsLong = null;
124-
try {
125-
ageAsLong = Long.parseLong(ageHeader);
126-
} catch (NumberFormatException nfe) {
127-
// Ignore
128-
}
129-
if (ageAsLong != null && ageAsLong >= 0) {
130-
String evidence = "Age: " + ageHeader;
131-
LOGGER.debug(
132-
"{} was served from a HTTP/1.1 cache, due to presence of a valid (non-negative decimal integer) 'Age' response header value",
133-
msg.getRequestHeader().getURI());
134-
buildAlert(evidence, true).raise();
135-
return;
136-
}
105+
// The "Age" header (defined in RFC 7234) conveys the sender's estimate of the amount of
106+
// time since the response (or its revalidation) was generated at the origin server.
107+
// An HTTP/1.1 server that includes a cache MUST include an Age header field in every
108+
// response generated from its own cache.
109+
// i.e.: a valid "Age" header implies that the response was served from a cache
110+
// lets validate that it is actually a non-negative decimal integer, as mandated by RFC
111+
// 7234, however.
112+
// if there are multiple "Age" headers, just look for one valid value in the multiple
113+
// "Age" headers.. Not sure if this case is strictly valid with the spec, however.
114+
// Note: HTTP/1.0 caches do not implement "Age", so the absence of an "Age" header does
115+
// *not* imply that the response was served from the origin server, rather than a
116+
// cache..
117+
List<String> ageHeaders = msg.getResponseHeader().getHeaderValues("Age");
118+
if (!ageHeaders.isEmpty()) {
119+
for (String ageHeader : ageHeaders) {
120+
LOGGER.trace("Validating Age header value [{}]", ageHeader);
121+
Long ageAsLong = null;
122+
try {
123+
ageAsLong = Long.parseLong(ageHeader);
124+
} catch (NumberFormatException nfe) {
125+
// Ignore
126+
}
127+
if (ageAsLong != null && ageAsLong >= 0) {
128+
String evidence = "Age: " + ageHeader;
129+
LOGGER.debug(
130+
"{} was served from a HTTP/1.1 cache, due to presence of a valid (non-negative decimal integer) 'Age' response header value",
131+
msg.getRequestHeader().getURI());
132+
buildAlert(evidence, true).raise();
133+
return;
137134
}
138135
}
139-
140-
} catch (Exception e) {
141-
LOGGER.error("An error occurred while checking if a URL was served from a cache", e);
142136
}
143137
}
144138

‎addOns/pscanrulesAlpha/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
55

66
## Unreleased
7-
7+
### Changed
8+
- Reduced usage of error level logging.
89

910
## [45] - 2025-06-20
1011
### Added

‎addOns/pscanrulesAlpha/src/main/java/org/zaproxy/zap/extension/pscanrulesAlpha/ExampleFilePassiveScanRule.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ private static List<String> loadFile(String file) {
122122
BufferedReader reader = null;
123123
File f = new File(Constant.getZapHome() + File.separator + file);
124124
if (!f.exists()) {
125-
LOGGER.error("No such file: {}", f.getAbsolutePath());
125+
LOGGER.warn("No such file: {}", f.getAbsolutePath());
126126
return strings;
127127
}
128128
try {
@@ -134,7 +134,7 @@ private static List<String> loadFile(String file) {
134134
}
135135
}
136136
} catch (IOException e) {
137-
LOGGER.error(
137+
LOGGER.debug(
138138
"Error on opening/reading example error file. Error: {}", e.getMessage(), e);
139139
} finally {
140140
if (reader != null) {

‎addOns/pscanrulesBeta/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
55

66
## Unreleased
7-
7+
### Changed
8+
- Reduced usage of error level logging.
89

910
## [44] - 2025-06-20
1011
### Changed

0 commit comments

Comments
 (0)