Skip to content

Commit 53b3a6f

Browse files
psiinonpsiinon
authored
Merge pull request #5252 from kingthorin/retire-vid
retire: Only target relevant responses
2 parents cd47a06 + 538f5d7 commit 53b3a6f

File tree

3 files changed

+46
-54
lines changed

3 files changed

+46
-54
lines changed

Diff for: ‎addOns/retire/CHANGELOG.md

+2-19
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,21 @@ 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+
- Now only targets relevant responses (HTML and JS).
89

910
## [0.29.0] - 2024-01-03
1011
### Changed
1112
- Updated with upstream retire.js pattern changes.
1213

13-
14-
1514
## [0.28.0] - 2023-12-04
1615
### Changed
1716
- Updated with upstream retire.js pattern changes.
1817

19-
20-
2118
## [0.27.0] - 2023-11-03
2219
### Changed
2320
- Updated with upstream retire.js pattern changes.
2421

25-
26-
2722
## [0.26.0] - 2023-10-12
2823
### Changed
2924
- Update minimum ZAP version to 2.14.0.
@@ -39,26 +34,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
3934
- Update minimum ZAP version to 2.13.0.
4035
- Updated with upstream retire.js pattern changes.
4136

42-
43-
4437
## [0.23.0] - 2023-06-02
4538
### Changed
4639
- Updated with upstream retire.js pattern changes.
4740

48-
49-
5041
## [0.22.0] - 2023-05-03
5142
### Changed
5243
- Updated with upstream retire.js pattern changes.
5344

54-
55-
5645
## [0.21.0] - 2023-04-04
5746
### Changed
5847
- Updated with upstream retire.js pattern changes.
5948

60-
61-
6249
## [0.20.0] - 2023-03-03
6350
### Changed
6451
- Updated with upstream retire.js pattern changes.
@@ -73,14 +60,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
7360
### Changed
7461
- Updated with upstream retire.js pattern changes.
7562

76-
77-
7863
## [0.17.0] - 2022-11-14
7964
### Changed
8065
- Updated with upstream retire.js pattern changes.
8166

82-
83-
8467
## [0.16.0] - 2022-10-27
8568
### Changed
8669
- Update minimum ZAP version to 2.12.0.

Diff for: ‎addOns/retire/src/main/java/org/zaproxy/addon/retire/RetireScanRule.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import org.parosproxy.paros.core.scanner.Alert;
3434
import org.parosproxy.paros.network.HttpMessage;
3535
import org.zaproxy.addon.commonlib.CommonAlertTag;
36-
import org.zaproxy.addon.commonlib.ResourceIdentificationUtils;
3736
import org.zaproxy.addon.retire.model.Repo;
3837
import org.zaproxy.zap.extension.pscan.PluginPassiveScanner;
3938

@@ -61,16 +60,15 @@ public int getPluginId() {
6160

6261
@Override
6362
public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
64-
if (!getHelper().isPage200(msg) || getRepo() == null) {
65-
return;
66-
}
67-
String uri = msg.getRequestHeader().getURI().toString();
68-
if (!ResourceIdentificationUtils.isImage(msg) && !ResourceIdentificationUtils.isCss(msg)) {
69-
Repo scanRepo = getRepo();
63+
Repo scanRepo = getRepo();
64+
if (!getHelper().isPage200(msg) || scanRepo == null) {
7065
if (scanRepo == null) {
7166
LOGGER.error("\tThe Retire.js repository was null.");
72-
return;
7367
}
68+
return;
69+
}
70+
String uri = msg.getRequestHeader().getURI().toString();
71+
if (msg.getResponseHeader().isHtml() || msg.getResponseHeader().isJavaScript()) {
7472
Result result = scanRepo.scanJS(msg, source);
7573
if (result == null) {
7674
LOGGER.debug("\tNo vulnerabilities found in record {} with URL {}", id, uri);

Diff for: ‎addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireScanRuleUnitTest.java

+38-27
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,21 @@
2727
import static org.mockito.Mockito.any;
2828

2929
import java.io.IOException;
30+
import java.util.List;
3031
import java.util.Map;
3132
import org.apache.commons.httpclient.URI;
3233
import org.apache.commons.httpclient.URIException;
3334
import org.junit.jupiter.api.Test;
3435
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.CsvSource;
3537
import org.junit.jupiter.params.provider.ValueSource;
38+
import org.parosproxy.paros.core.scanner.Alert;
3639
import org.parosproxy.paros.network.HttpHeader;
3740
import org.parosproxy.paros.network.HttpMalformedHeaderException;
3841
import org.parosproxy.paros.network.HttpMessage;
3942
import org.parosproxy.paros.network.HttpRequestHeader;
4043
import org.zaproxy.addon.commonlib.CommonAlertTag;
44+
import org.zaproxy.addon.commonlib.http.HttpFieldsNames;
4145
import org.zaproxy.addon.retire.model.Repo;
4246

4347
class RetireScanRuleUnitTest extends PassiveScannerTest<RetireScanRule> {
@@ -66,34 +70,22 @@ void shouldIgnoreNon200OkMessages() {
6670
assertEquals(0, alertsRaised.size());
6771
}
6872

69-
@Test
70-
void shouldIgnoreCssUrl() {
71-
// Given
72-
HttpMessage msg = createMessage("https://www.example.com/assets/styles.css", null);
73-
given(passiveScanData.isPage200(any())).willReturn(true);
74-
// When
75-
scanHttpResponseReceive(msg);
76-
// Then
77-
assertEquals(0, alertsRaised.size());
78-
}
79-
80-
@Test
81-
void shouldIgnoreCssResponse() {
82-
// Given
83-
HttpMessage msg = createMessage("https://www.example.com/assets/styles.scss", null);
84-
msg.getResponseHeader().addHeader(HttpHeader.CONTENT_TYPE, "text/css");
85-
given(passiveScanData.isPage200(any())).willReturn(true);
86-
// When
87-
scanHttpResponseReceive(msg);
88-
// Then
89-
assertEquals(0, alertsRaised.size());
90-
}
91-
92-
@Test
93-
void shouldIgnoreImageResponse() {
73+
@ParameterizedTest
74+
@CsvSource({
75+
"text/css, style.css",
76+
"text/css, style.scss",
77+
"'', style.css",
78+
"text/css, ''",
79+
"text/css, styles",
80+
"video/mp4, foo.mp4",
81+
"image/gif, ''",
82+
"image/gif, foo.gif",
83+
"'', image/gif"
84+
})
85+
void shouldIgnoreIrrelevantResponseContentTypes(String contentType, String file) {
9486
// Given
95-
HttpMessage msg = createMessage("https://www.example.com/assets/image.gif", null);
96-
msg.getResponseHeader().addHeader(HttpHeader.CONTENT_TYPE, "image/gif");
87+
HttpMessage msg = createMessage("https://www.example.com/assets/" + file, null);
88+
msg.getResponseHeader().setHeader(HttpHeader.CONTENT_TYPE, contentType);
9789
given(passiveScanData.isPage200(any())).willReturn(true);
9890
// When
9991
scanHttpResponseReceive(msg);
@@ -106,6 +98,7 @@ void shouldRaiseAlertOnVulnerableUrl() {
10698
// Given
10799
HttpMessage msg =
108100
createMessage("http://example.com/ajax/libs/angularjs/1.2.19/angular.min.js", null);
101+
msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/javascript");
109102
given(passiveScanData.isPage200(any())).willReturn(true);
110103
// When
111104
scanHttpResponseReceive(msg);
@@ -122,6 +115,7 @@ void shouldRaiseAlertOnVulnerableUrl() {
122115
void shouldRaiseAlertOnVulnerableFilename(String fileName) {
123116
// Given
124117
HttpMessage msg = createMessage("http://example.com/CommonElements/js/" + fileName, null);
118+
msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/javascript");
125119
given(passiveScanData.isPage200(any())).willReturn(true);
126120
// When
127121
scanHttpResponseReceive(msg);
@@ -143,6 +137,7 @@ void shouldRaiseAlertOnVulnerableContent() {
143137
+ " * Licensed under the MIT license\n"
144138
+ " */";
145139
HttpMessage msg = createMessage("http://example.com/angular.min.js", content);
140+
msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/javascript");
146141
given(passiveScanData.isPage200(any())).willReturn(true);
147142
// When
148143
scanHttpResponseReceive(msg);
@@ -166,6 +161,7 @@ void shouldRaiseAlertOnHashOfVulnerableContent() {
166161
+ " * Licensed under the MIT license\n"
167162
+ " */";
168163
HttpMessage msg = createMessage("http://example.com/hash.js", content);
164+
msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/javascript");
169165
given(passiveScanData.isPage200(any())).willReturn(true);
170166
// When
171167
scanHttpResponseReceive(msg);
@@ -212,6 +208,20 @@ void shouldReturnExpectedMappings() {
212208
is(equalTo(CommonAlertTag.OWASP_2017_A09_VULN_COMP.getValue())));
213209
}
214210

211+
@Test
212+
void shouldHaveExpectedExampleAlert() {
213+
// Given / When
214+
List<Alert> alerts = rule.getExampleAlerts();
215+
// Then
216+
assertThat(alerts.size(), is(equalTo(1)));
217+
}
218+
219+
@Test
220+
@Override
221+
public void shouldHaveValidReferences() {
222+
super.shouldHaveValidReferences();
223+
}
224+
215225
private HttpMessage createMessage(String url, String body) {
216226
HttpMessage msg = new HttpMessage();
217227
if (url == null) {
@@ -233,6 +243,7 @@ private HttpMessage createMessage(String url, String body) {
233243
} catch (HttpMalformedHeaderException e) {
234244
// Nothing to do
235245
}
246+
msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/html");
236247
msg.setResponseBody(body);
237248

238249
return msg;

0 commit comments

Comments
 (0)