Skip to content

Commit a67eae9

Browse files
authored
asacnrules: Hidden files FP when 200s returned for 404s (#6719)
* asacnrules: Hidden files FP when 200s returned for 404s Fixes zaproxy/zaproxy#8434 Signed-off-by: Simon Bennetts <[email protected]>
1 parent f31e803 commit a67eae9

File tree

3 files changed

+115
-44
lines changed

3 files changed

+115
-44
lines changed

addOns/ascanrules/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+
### Fixed
8+
- Hidden Files rule raising false positives if server returning 200 for files that don't exist (Issue 8434).
89

910
## [73] - 2025-09-02
1011
### Changed

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/HiddenFilesScanRule.java

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.HashMap;
3030
import java.util.List;
3131
import java.util.Map;
32+
import java.util.Random;
3233
import java.util.function.Supplier;
3334
import net.sf.json.JSONArray;
3435
import net.sf.json.JSONException;
@@ -48,6 +49,7 @@
4849
import org.parosproxy.paros.network.HttpStatusCode;
4950
import org.zaproxy.addon.commonlib.CommonAlertTag;
5051
import org.zaproxy.addon.commonlib.PolicyTag;
52+
import org.zaproxy.addon.commonlib.http.ComparableResponse;
5153
import org.zaproxy.addon.commonlib.http.HttpFieldsNames;
5254

5355
/**
@@ -67,6 +69,7 @@ public class HiddenFilesScanRule extends AbstractHostPlugin implements CommonAct
6769
private static final int PLUGIN_ID = 40035;
6870
private static final Logger LOGGER = LogManager.getLogger(HiddenFilesScanRule.class);
6971
private static final Map<String, String> ALERT_TAGS;
72+
private static final Random staticRandomGenerator = new Random();
7073

7174
static {
7275
Map<String, String> alertTags =
@@ -107,20 +110,32 @@ public int getId() {
107110
public void init() {
108111
hfList = readFromJsonFile(DEFAULT_PAYLOAD_PATH);
109112
for (String payload : getHiddenFilePayloads().get()) {
110-
hfList.add(
111-
new HiddenFile(
112-
payload,
113-
Collections.emptyList(),
114-
Collections.emptyList(),
115-
"",
116-
Collections.emptyList(),
117-
"",
118-
true));
113+
hfList.add(new HiddenFile(payload, true));
119114
}
120115
}
121116

117+
/** Copied from org.parosproxy.paros.core.scanner.Analyser */
118+
private static long getRndPositiveLong() {
119+
long rnd = Long.MIN_VALUE;
120+
while (rnd == Long.MIN_VALUE) {
121+
rnd = staticRandomGenerator.nextLong();
122+
}
123+
return Math.abs(rnd);
124+
}
125+
122126
@Override
123127
public void scan() {
128+
HttpMessage willNotExistMsg =
129+
sendHiddenFileRequest(new HiddenFile("zap" + getRndPositiveLong(), true));
130+
HttpMessage willNotExistDotMsg =
131+
sendHiddenFileRequest(new HiddenFile(".zap" + getRndPositiveLong(), true));
132+
if (willNotExistMsg == null || willNotExistDotMsg == null) {
133+
return;
134+
}
135+
ComparableResponse willNotExistCompResp = new ComparableResponse(willNotExistMsg, null);
136+
ComparableResponse willNotExistDotCompResp =
137+
new ComparableResponse(willNotExistDotMsg, null);
138+
124139
for (HiddenFile file : hfList) {
125140

126141
if (isStop()) {
@@ -134,6 +149,17 @@ public void scan() {
134149
}
135150
int statusCode = testMsg.getResponseHeader().getStatusCode();
136151
if (isPage200(testMsg)) {
152+
ComparableResponse cr = new ComparableResponse(testMsg, null);
153+
ComparableResponse testCr =
154+
file.getPath().startsWith(".")
155+
? willNotExistDotCompResp
156+
: willNotExistCompResp;
157+
158+
if (cr.compareWith(testCr) >= 0.9) {
159+
// Getting essentially the same response for a file that really doesn't exist
160+
continue;
161+
}
162+
137163
String responseBody = testMsg.getResponseBody().toString();
138164
boolean matches =
139165
doesNotMatch(responseBody, file.getNotContent())
@@ -453,6 +479,17 @@ static class HiddenFile {
453479
private final String extra;
454480
private final boolean custom;
455481

482+
public HiddenFile(String path, boolean custom) {
483+
this(
484+
path,
485+
Collections.emptyList(),
486+
Collections.emptyList(),
487+
"",
488+
Collections.emptyList(),
489+
"",
490+
custom);
491+
}
492+
456493
public HiddenFile(
457494
String path,
458495
List<String> content,

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/HiddenFilesScanRuleUnitTest.java

Lines changed: 67 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.Collections;
4242
import java.util.List;
4343
import java.util.Map;
44+
import java.util.Random;
4445
import org.junit.jupiter.api.AfterEach;
4546
import org.junit.jupiter.api.Test;
4647
import org.junit.jupiter.params.ParameterizedTest;
@@ -185,10 +186,11 @@ void shouldRaiseAlertIfTestedUrlRespondsOkWithRelevantContent()
185186
false);
186187

187188
this.nano.addHandler(new OkResponse(servePath));
189+
nano.setHandler404(new OkWithRndToken(""));
188190
this.nano.addHandler(
189191
new StaticContentServerHandler(
190192
'/' + testPath,
191-
"<html><head></head><H>Awesome Title</H1> Some Text... <html>"));
193+
"<html><head></head><H1>Awesome Title</H1> Some Text... <html>"));
192194

193195
HttpMessage msg = this.getHttpMessage(servePath);
194196

@@ -426,19 +428,39 @@ void shouldRaiseAlertWithHighConfidenceIfContentStringsAllMatch()
426428
}
427429

428430
@Test
429-
void shouldRaiseAlertWithLowConfidenceIfTestedUrlRespondsOkToCustomPayload()
430-
throws HttpMalformedHeaderException {
431+
void shouldNotRaiseAlertIfMajorityResponsesTooSimilar() throws HttpMalformedHeaderException {
432+
// Given
433+
String servePath = "/shouldNotAlert";
434+
435+
String testPath = "foo/test.php";
436+
List<String> customPaths = Arrays.asList(testPath);
437+
438+
nano.setHandler404(new OkWithRndToken(""));
439+
440+
HttpMessage msg = this.getHttpMessage(servePath);
441+
442+
HiddenFilesScanRule.setPayloadProvider(() -> customPaths);
443+
rule.init(msg, this.parent);
444+
445+
// When
446+
rule.scan();
447+
448+
// Then
449+
assertThat(alertsRaised, hasSize(0));
450+
}
451+
452+
@Test
453+
void shouldtRaiseAlertForMatchWith404As200() throws HttpMalformedHeaderException {
431454
// Given
432455
String servePath = "/shouldAlert";
433456

434457
String testPath = "foo/test.php";
435458
List<String> customPaths = Arrays.asList(testPath);
436459

437460
this.nano.addHandler(new OkResponse(servePath));
438-
this.nano.addHandler(
439-
new StaticContentServerHandler(
440-
'/' + testPath,
441-
"<html><head></head><H>Awesome Title</H1> Some Text... <html>"));
461+
this.nano.addHandler(new OkResponse("/" + testPath));
462+
463+
nano.setHandler404(new OkWithRndToken(""));
442464

443465
HttpMessage msg = this.getHttpMessage(servePath);
444466

@@ -447,15 +469,33 @@ void shouldRaiseAlertWithLowConfidenceIfTestedUrlRespondsOkToCustomPayload()
447469

448470
// When
449471
rule.scan();
472+
450473
// Then
451474
assertThat(alertsRaised, hasSize(1));
452475
Alert alert = alertsRaised.get(0);
453-
assertEquals(1, httpMessagesSent.size());
476+
assertThat(httpMessagesSent, hasSize(greaterThanOrEqualTo(1)));
454477
assertEquals(Alert.RISK_MEDIUM, alertsRaised.get(0).getRisk());
455478
assertEquals(Alert.CONFIDENCE_LOW, alertsRaised.get(0).getConfidence());
456479
assertEquals(rule.getReference(), alert.getReference());
457480
}
458481

482+
@ParameterizedTest
483+
@MethodSource("org.zaproxy.zap.extension.ascanrules.HiddenFilesScanRule#getHiddenFiles()")
484+
void shouldNotRaiseAlertIfMajorityResponsesTooSimilarForBuiltInCustomPayloads(String fileName)
485+
throws HttpMalformedHeaderException {
486+
// Given
487+
String servePath = "/shouldNotAlert";
488+
489+
nano.setHandler404(new OkWithRndToken(""));
490+
491+
rule.init(getHttpMessage(servePath), parent);
492+
493+
// When
494+
rule.scan();
495+
// Then
496+
assertThat(alertsRaised, hasSize(0));
497+
}
498+
459499
@Test
460500
void shouldNotRaiseAlertIfResponseStatusIsNotOkOrAuthRelated()
461501
throws HttpMalformedHeaderException {
@@ -756,32 +796,6 @@ void shouldReturnExpectedExampleAlert() {
756796
assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
757797
}
758798

759-
@ParameterizedTest
760-
@MethodSource("org.zaproxy.zap.extension.ascanrules.HiddenFilesScanRule#getHiddenFiles()")
761-
// XXX A very likely FP.
762-
void shouldRaiseAlertIfTestedUrlRespondsOkForCustomPayloads(String fileName)
763-
throws HttpMalformedHeaderException {
764-
// Given
765-
String servePath = "/shouldAlert";
766-
nano.addHandler(new OkResponse(servePath));
767-
nano.addHandler(
768-
new StaticContentServerHandler(
769-
'/' + fileName,
770-
"<html><head></head><H>Awesome Title</H1> Some Text... <html>"));
771-
rule.init(getHttpMessage(servePath), parent);
772-
773-
// When
774-
rule.scan();
775-
// Then
776-
assertThat(alertsRaised, hasSize(1));
777-
Alert alert = alertsRaised.get(0);
778-
assertThat(httpMessagesSent, hasSize(greaterThanOrEqualTo(1)));
779-
assertThat(alert.getRisk(), is(equalTo(Alert.RISK_MEDIUM)));
780-
assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
781-
assertThat(alert.getEvidence(), is(equalTo("HTTP/1.1 200 OK")));
782-
assertThat(alert.getOtherInfo(), is(equalTo("")));
783-
}
784-
785799
@Test
786800
@Override
787801
public void shouldHaveValidReferences() {
@@ -868,4 +882,23 @@ public OkBinResponse(String path, String content) {
868882
super(path, content);
869883
}
870884
}
885+
886+
private static class OkWithRndToken extends NanoServerHandler {
887+
888+
private Random rnd = new Random();
889+
890+
public OkWithRndToken(String name) {
891+
super(name);
892+
}
893+
894+
@Override
895+
protected Response serve(IHTTPSession session) {
896+
return NanoHTTPD.newFixedLengthResponse(
897+
Response.Status.OK,
898+
"text/html",
899+
"<html><head></head><body><H1>Awesome Title</H1> Some Text... <br>"
900+
+ rnd.nextLong()
901+
+ "</body></html>");
902+
}
903+
}
871904
}

0 commit comments

Comments
 (0)