Skip to content

Commit 1e7ebe5

Browse files
psiinonpsiinon
authored
Merge pull request #5261 from kingthorin/si-more-examples
pscanrules: More example alerts
2 parents 99d8ce1 + b7ccd1b commit 1e7ebe5

11 files changed

+195
-64
lines changed

‎addOns/pscanrules/CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1010
- Timestamp Disclosure - Unix
1111
- Hash Disclosure
1212
- Cross-Domain Misconfiguration
13+
- Weak Authentication Method
14+
- Reverse Tabnabbing
15+
- CSRF Countermeasures
16+
- The following scan rules now have alert references (Issue 7100):
17+
- Weak Authentication Method
1318
- The references for Alerts from the following rules were also updated (Issue 8262):
1419
- Timestamp Disclosure - Unix
1520
- Hash Disclosure
1621
- View State Scan Rule
22+
- Weak Authentication Method
1723

1824
## [55] - 2024-01-26
1925
### Changed

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

+32-16
Original file line numberDiff line numberDiff line change
@@ -208,34 +208,25 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
208208

209209
int risk = Alert.RISK_MEDIUM;
210210
String desc = Constant.messages.getString("pscanrules.noanticsrftokens.desc");
211-
String extraInfo =
212-
Constant.messages.getString(
213-
"pscanrules.noanticsrftokens.alert.extrainfo",
214-
tokenNamesFlattened,
215-
formDetails);
211+
String extraInfo = getExtraInfo(tokenNamesFlattened, formDetails);
216212
if (hasSecurityAnnotation) {
217213
risk = Alert.RISK_INFO;
218214
extraInfo =
219215
Constant.messages.getString(
220216
"pscanrules.noanticsrftokens.extrainfo.annotation");
221217
}
222218

223-
newAlert()
224-
.setRisk(risk)
225-
.setConfidence(Alert.CONFIDENCE_LOW)
226-
.setDescription(desc + "\n" + getDescription())
227-
.setOtherInfo(extraInfo)
228-
.setSolution(getSolution())
229-
.setReference(getReference())
230-
.setEvidence(evidence)
231-
.setCweId(getCweId())
232-
.setWascId(getWascId())
233-
.raise();
219+
buildAlert(risk, desc, extraInfo, evidence).raise();
234220
}
235221
}
236222
LOGGER.debug("\tScan of record {} took {} ms", id, System.currentTimeMillis() - start);
237223
}
238224

225+
private String getExtraInfo(String tokenNamesFlattened, String formDetails) {
226+
return Constant.messages.getString(
227+
"pscanrules.noanticsrftokens.alert.extrainfo", tokenNamesFlattened, formDetails);
228+
}
229+
239230
private boolean formOnIgnoreList(Element formElement, List<String> ignoreList) {
240231
String id = formElement.getAttributeValue("id");
241232
String name = formElement.getAttributeValue("name");
@@ -293,6 +284,31 @@ public int getWascId() {
293284
return 9;
294285
}
295286

287+
private AlertBuilder buildAlert(int risk, String desc, String extraInfo, String evidence) {
288+
return newAlert()
289+
.setRisk(risk)
290+
.setConfidence(Alert.CONFIDENCE_LOW)
291+
.setDescription(desc + "\n" + getDescription())
292+
.setOtherInfo(extraInfo)
293+
.setSolution(getSolution())
294+
.setReference(getReference())
295+
.setEvidence(evidence)
296+
.setCweId(getCweId())
297+
.setWascId(getWascId());
298+
}
299+
300+
@Override
301+
public List<Alert> getExampleAlerts() {
302+
return List.of(
303+
buildAlert(
304+
Alert.RISK_MEDIUM,
305+
Constant.messages.getString("pscanrules.noanticsrftokens.desc"),
306+
getExtraInfo(
307+
"[token, csrfToken, csrf-token]", "[Form 1: \"name\" ]"),
308+
"<form name=\"someName\" data-no-csrf>")
309+
.build());
310+
}
311+
296312
protected ExtensionAntiCSRF getExtensionAntiCSRF() {
297313
if (extensionAntiCSRF == null) {
298314
return Control.getSingleton()

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

+51-30
Original file line numberDiff line numberDiff line change
@@ -183,27 +183,8 @@ public void scanHttpRequestSend(HttpMessage msg, int id) {
183183
digestInfo = authValues[1]; // info to output in the logging message.
184184
}
185185

186-
newAlert()
187-
.setName(
188-
Constant.messages.getString(
189-
"pscanrules.authenticationcredentialscaptured.name"))
190-
.setRisk(alertRisk)
191-
.setConfidence(alertConf)
192-
.setDescription(
193-
Constant.messages.getString(
194-
"pscanrules.authenticationcredentialscaptured.desc"))
195-
.setOtherInfo(extraInfo)
196-
.setSolution(
197-
Constant.messages.getString(
198-
"pscanrules.authenticationcredentialscaptured.soln"))
199-
.setReference(
200-
Constant.messages.getString(
201-
"pscanrules.authenticationcredentialscaptured.refs"))
202-
.setCweId(287) // CWE Id - Improper authentication
203-
.setWascId(1) // WASC Id - Insufficient authentication
204-
.raise();
186+
buildCapturedAlert(alertRisk, alertConf, extraInfo).raise();
205187

206-
// and log it, without internationalising it.
207188
LOGGER.info(
208189
"Authentication Credentials were captured. [{}] [{}] uses insecure authentication mechanism [{}], revealing username [{}] and password/additional information [{}]",
209190
method,
@@ -269,18 +250,58 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
269250
for (String auth : authHeaders) {
270251
if (auth.toLowerCase().indexOf("basic") > -1
271252
|| auth.toLowerCase().indexOf("digest") > -1) {
272-
newAlert()
273-
.setRisk(Alert.RISK_MEDIUM)
274-
.setConfidence(Alert.CONFIDENCE_MEDIUM)
275-
.setDescription(getDescription())
276-
.setSolution(getSolution())
277-
.setReference(getReference())
278-
.setEvidence(HttpHeader.WWW_AUTHENTICATE + ": " + auth)
279-
.setCweId(getCweId())
280-
.setWascId(getWascId())
281-
.raise();
253+
buildAlert(auth).raise();
282254
}
283255
}
284256
}
285257
}
258+
259+
private AlertBuilder buildCapturedAlert(int alertRisk, int alertConf, String extraInfo) {
260+
return newAlert()
261+
.setName(
262+
Constant.messages.getString(
263+
"pscanrules.authenticationcredentialscaptured.name"))
264+
.setRisk(alertRisk)
265+
.setConfidence(alertConf)
266+
.setDescription(
267+
Constant.messages.getString(
268+
"pscanrules.authenticationcredentialscaptured.desc"))
269+
.setOtherInfo(extraInfo)
270+
.setSolution(
271+
Constant.messages.getString(
272+
"pscanrules.authenticationcredentialscaptured.soln"))
273+
.setReference(
274+
Constant.messages.getString(
275+
"pscanrules.authenticationcredentialscaptured.refs"))
276+
.setCweId(287) // CWE Id - Improper authentication
277+
.setWascId(1) // WASC Id - Insufficient authentication
278+
.setAlertRef(getPluginId() + "-1");
279+
}
280+
281+
private AlertBuilder buildAlert(String auth) {
282+
return newAlert()
283+
.setRisk(Alert.RISK_MEDIUM)
284+
.setConfidence(Alert.CONFIDENCE_MEDIUM)
285+
.setDescription(getDescription())
286+
.setSolution(getSolution())
287+
.setReference(getReference())
288+
.setEvidence(HttpHeader.WWW_AUTHENTICATE + ": " + auth)
289+
.setCweId(getCweId())
290+
.setWascId(getWascId())
291+
.setAlertRef(getPluginId() + "-2");
292+
}
293+
294+
@Override
295+
public List<Alert> getExampleAlerts() {
296+
return List.of(
297+
buildCapturedAlert(
298+
Alert.RISK_MEDIUM,
299+
Alert.CONFIDENCE_MEDIUM,
300+
"[POST] "
301+
+ "[http://www.example.com] uses insecure authentication mechanism [Digest], "
302+
+ "revealing username [admin] and additional information "
303+
+ "[username=\"admin\", realm=\"members only\"].")
304+
.build(),
305+
buildAlert("Basic realm=\"Private\"").build());
306+
}
286307
}

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

+19-8
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,23 @@ private boolean checkElement(Element link) {
130130
if (relAtt != null) {
131131
relAtt = relAtt.toLowerCase();
132132
if (relAtt.contains(OPENER) && !relAtt.contains(NOOPENER)) {
133-
newAlert()
134-
.setRisk(Alert.RISK_MEDIUM)
135-
.setConfidence(Alert.CONFIDENCE_MEDIUM)
136-
.setDescription(getDescription())
137-
.setSolution(getSolution())
138-
.setReference(getReference())
139-
.setEvidence(link.toString())
140-
.raise();
133+
buildAlert(link.toString()).raise();
141134
return true;
142135
}
143136
}
144137
return false;
145138
}
146139

140+
private AlertBuilder buildAlert(String evidence) {
141+
return newAlert()
142+
.setRisk(Alert.RISK_MEDIUM)
143+
.setConfidence(Alert.CONFIDENCE_MEDIUM)
144+
.setDescription(getDescription())
145+
.setSolution(getSolution())
146+
.setReference(getReference())
147+
.setEvidence(evidence);
148+
}
149+
147150
@Override
148151
public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
149152
if (msg.getResponseBody().length() == 0 || !msg.getResponseHeader().isHtml()) {
@@ -190,4 +193,12 @@ private String getReference() {
190193
public Map<String, String> getAlertTags() {
191194
return ALERT_TAGS;
192195
}
196+
197+
@Override
198+
public List<Alert> getExampleAlerts() {
199+
return List.of(
200+
buildAlert(
201+
"<a href=\"https://www.example3.com/page1\" rel=\"opener\" target=\"_blank\">link</a>")
202+
.build());
203+
}
193204
}

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

+20-8
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,20 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
8484

8585
if (evidence != null && evidence.length() > 0) {
8686
// we found something
87-
newAlert()
88-
.setRisk(Alert.RISK_INFO)
89-
.setConfidence(Alert.CONFIDENCE_MEDIUM)
90-
.setDescription(getDescription())
91-
.setOtherInfo(otherInfo)
92-
.setSolution(getSolution())
93-
.setEvidence(evidence)
94-
.raise();
87+
buildAlert(otherInfo, evidence).raise();
9588
}
9689
}
9790

91+
private AlertBuilder buildAlert(String otherInfo, String evidence) {
92+
return newAlert()
93+
.setRisk(Alert.RISK_INFO)
94+
.setConfidence(Alert.CONFIDENCE_MEDIUM)
95+
.setDescription(getDescription())
96+
.setOtherInfo(otherInfo)
97+
.setSolution(getSolution())
98+
.setEvidence(evidence);
99+
}
100+
98101
@Override
99102
public int getPluginId() {
100103
return 10109;
@@ -107,4 +110,13 @@ private String getDescription() {
107110
private String getSolution() {
108111
return Constant.messages.getString(MESSAGE_PREFIX + "soln");
109112
}
113+
114+
@Override
115+
public List<Alert> getExampleAlerts() {
116+
return List.of(
117+
buildAlert(
118+
Constant.messages.getString(MESSAGE_PREFIX + "other.self"),
119+
"<a href=\"link\" target=\"_self\">Link</a>")
120+
.build());
121+
}
110122
}

‎addOns/pscanrules/src/main/resources/org/zaproxy/zap/extension/pscanrules/resources/Messages.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pscanrules.authenticationcredentialscaptured.alert.basicauth.extrainfo = [{0}] [
2525
pscanrules.authenticationcredentialscaptured.alert.digestauth.extrainfo = [{0}] [{1}] uses insecure authentication mechanism [{2}], revealing username [{3}] and additional information [{4}].
2626
pscanrules.authenticationcredentialscaptured.desc = An insecure authentication mechanism is in use. This allows an attacker on the network access to the userid and password of the authenticated user. For Basic Authentication, the attacker must merely monitor the network traffic until a Basic Authentication request is received, and then base64 decode the username and password. For Digest Authentication, the attacker has access to the username, and possibly also the password, if the hash (including a nonce) can be successfully cracked, or if a Man-In-The-Middle attack is mounted.\nThe attacker eavesdrops on the network until an authentication has completed.
2727
pscanrules.authenticationcredentialscaptured.name = Authentication Credentials Captured
28-
pscanrules.authenticationcredentialscaptured.refs = https://owasp.org/www-community/attacks/Brute_force_attack\nhttp://en.wikipedia.org/wiki/Digest_access_authentication
28+
pscanrules.authenticationcredentialscaptured.refs = https://owasp.org/www-community/attacks/Brute_force_attack\nhttps://en.wikipedia.org/wiki/Digest_access_authentication
2929
pscanrules.authenticationcredentialscaptured.soln = Use HTTPS, and use a secure authentication mechanism that does not transmit the userid or password in an un-encrypted fashion. In particular, avoid use of the Basic Authentication mechanism, since this trivial obfuscation mechanism is easily broken.
3030
3131
pscanrules.bigredirects.desc = The server has responded with a redirect that seems to provide a large response. This may indicate that although the server sent a redirect it also responded with body content (which may include sensitive details, PII, etc.).

‎addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/CsrfCountermeasuresScanRuleUnitTest.java

+14
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,20 @@ void shouldReturnExpectedMappings() {
123123
is(equalTo(CommonAlertTag.WSTG_V42_SESS_05_CSRF.getValue())));
124124
}
125125

126+
@Test
127+
void shouldHaveExpectedExampleAlerts() {
128+
// Given / When
129+
List<Alert> alerts = rule.getExampleAlerts();
130+
// Then
131+
assertThat(alerts.size(), is(equalTo(1)));
132+
}
133+
134+
@Test
135+
@Override
136+
public void shouldHaveValidReferences() {
137+
super.shouldHaveValidReferences();
138+
}
139+
126140
@Test
127141
void shouldNotRaiseAlertIfContentTypeIsNotHTML() {
128142
// Given

‎addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/InsecureAuthenticationScanRuleUnitTest.java

+19
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import java.io.IOException;
2727
import java.util.Base64;
28+
import java.util.List;
2829
import java.util.Map;
2930
import org.apache.commons.httpclient.URI;
3031
import org.junit.jupiter.api.Test;
@@ -97,6 +98,24 @@ void shouldReturnExpectedMappings() {
9798
is(equalTo(CommonAlertTag.WSTG_V42_ATHN_01_CREDS_NO_CRYPTO.getValue())));
9899
}
99100

101+
@Test
102+
void shouldHaveExpectedExampleAlerts() {
103+
// Given / When
104+
List<Alert> alerts = rule.getExampleAlerts();
105+
// Then
106+
assertThat(alerts.size(), is(equalTo(2)));
107+
Alert capturedAlert = alerts.get(0);
108+
assertThat(capturedAlert.getAlertRef(), is(equalTo("10105-1")));
109+
Alert weakAlert = alerts.get(1);
110+
assertThat(weakAlert.getAlertRef(), is(equalTo("10105-2")));
111+
}
112+
113+
@Test
114+
@Override
115+
public void shouldHaveValidReferences() {
116+
super.shouldHaveValidReferences();
117+
}
118+
100119
@Test
101120
void shouldBeSecureIfHttpUsedWithSsl() throws HttpMalformedHeaderException {
102121
// Given

‎addOns/pscanrules/src/test/java/org/zaproxy/zap/extension/pscanrules/LinkTargetScanRuleUnitTest.java

+16
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@
2828

2929
import java.util.Arrays;
3030
import java.util.Collections;
31+
import java.util.List;
3132
import java.util.Map;
3233
import org.junit.jupiter.api.BeforeEach;
3334
import org.junit.jupiter.api.Test;
3435
import org.mockito.Mock;
3536
import org.mockito.Mock.Strictness;
37+
import org.parosproxy.paros.core.scanner.Alert;
3638
import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold;
3739
import org.parosproxy.paros.model.Model;
3840
import org.parosproxy.paros.model.Session;
@@ -492,4 +494,18 @@ void shouldReturnExpectedMappings() {
492494
tags.get(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getTag()),
493495
is(equalTo(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getValue())));
494496
}
497+
498+
@Test
499+
void shouldHaveExpectedExampleAlerts() {
500+
// Given / When
501+
List<Alert> alerts = rule.getExampleAlerts();
502+
// Then
503+
assertThat(alerts.size(), is(equalTo(1)));
504+
}
505+
506+
@Test
507+
@Override
508+
public void shouldHaveValidReferences() {
509+
super.shouldHaveValidReferences();
510+
}
495511
}

0 commit comments

Comments
 (0)