Skip to content

Commit bb49da3

Browse files
committed
scan rules: Clean code tweaks
- Add static modifier where applicable - Remove boiler plate or useless comments/JavaDoc attributes. Signed-off-by: kingthorin <[email protected]>
1 parent 5803df8 commit bb49da3

File tree

140 files changed

+338
-663
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

140 files changed

+338
-663
lines changed

addOns/ascanrules/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
99
- The following rules now includes example alert functionality for documentation generation purposes (Issue 6119), as well as now including Alert Tags (OWASP Top 10, WSTG, and updated CWE):
1010
- Server Side Template Injection
1111
- Server Side Template Injection (Blind)
12+
- Maintenance changes.
1213

1314
### Fixed
1415
- False positives in the Path Traversal rule.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public String getOther() {
101101
public void scan(HttpMessage msg, String param, String value) {
102102

103103
if (this.isStop()) { // Check if the user stopped things
104-
LOGGER.debug("Scanner {} Stopping.", this.getName());
104+
LOGGER.debug("Scan rule {} Stopping.", this.getName());
105105
return; // Stop!
106106
}
107107
if (isPage500(getBaseMsg())) // Check to see if the page closed initially
@@ -169,7 +169,7 @@ public int getWascId() {
169169
return 7;
170170
}
171171

172-
private String randomCharacterString(int length) {
172+
private static String randomCharacterString(int length) {
173173
StringBuilder sb1 = new StringBuilder(length + 1);
174174
int counter = 0;
175175
int character = 0;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin
282282
NIX_BLIND_OS_PAYLOADS.add("|" + insertedCMD + "#");
283283
}
284284

285-
// Logger instance
286285
private static final Logger LOGGER = LogManager.getLogger(CommandInjectionScanRule.class);
287286

288287
// Get WASC Vulnerability description
@@ -366,7 +365,7 @@ public int getRisk() {
366365
return Alert.RISK_HIGH;
367366
}
368367

369-
private String getOtherInfo(TestType testType, String testValue) {
368+
private static String getOtherInfo(TestType testType, String testValue) {
370369
return Constant.messages.getString(
371370
MESSAGE_PREFIX + "otherinfo." + testType.getNameKey(), testValue);
372371
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public String getReference() {
9595
return Constant.messages.getString(MESSAGE_PREFIX + "refs");
9696
}
9797

98-
private void checkIfDirectory(HttpMessage msg) throws URIException {
98+
private static void checkIfDirectory(HttpMessage msg) throws URIException {
9999

100100
URI uri = msg.getRequestHeader().getURI();
101101
uri.setQuery(null);

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

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,6 @@ public String getReference() {
147147
return VULN.getReferencesAsString();
148148
}
149149

150-
/**
151-
* Scan for External Redirect vulnerabilities
152-
*
153-
* @param msg a request only copy of the original message (the response isn't copied)
154-
* @param param the parameter name that need to be exploited
155-
* @param value the original parameter value
156-
*/
157150
@Override
158151
public void scan(HttpMessage msg, String param, String value) {
159152

@@ -342,7 +335,7 @@ private static boolean isRedirectHost(String value, boolean escaped) throws URIE
342335
* @param msg the current message where reflected redirection should be check into
343336
* @return get back the redirection type if exists
344337
*/
345-
private int isRedirected(String payload, HttpMessage msg) {
338+
private static int isRedirected(String payload, HttpMessage msg) {
346339

347340
// (1) Check if redirection by "Location" header
348341
// http://en.wikipedia.org/wiki/HTTP_location
@@ -471,7 +464,7 @@ private static boolean isRedirectPresent(Pattern pattern, String value) {
471464
* @param type the redirection type
472465
* @return a string representing the reason of this redirection
473466
*/
474-
private String getRedirectionReason(int type) {
467+
private static String getRedirectionReason(int type) {
475468
switch (type) {
476469
case REDIRECT_LOCATION_HEADER:
477470
return Constant.messages.getString(MESSAGE_PREFIX + "reason.location.header");
@@ -493,11 +486,6 @@ private String getRedirectionReason(int type) {
493486
}
494487
}
495488

496-
/**
497-
* Give back the risk associated to this vulnerability (high)
498-
*
499-
* @return the risk according to the Alert enum
500-
*/
501489
@Override
502490
public int getRisk() {
503491
return Alert.RISK_HIGH;
@@ -508,24 +496,14 @@ public Map<String, String> getAlertTags() {
508496
return ALERT_TAGS;
509497
}
510498

511-
/**
512-
* http://cwe.mitre.org/data/definitions/601.html
513-
*
514-
* @return the official CWE id
515-
*/
516499
@Override
517500
public int getCweId() {
518-
return 601;
501+
return 601; // http://cwe.mitre.org/data/definitions/601.html
519502
}
520503

521-
/**
522-
* http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse
523-
*
524-
* @return the official WASC id
525-
*/
526504
@Override
527505
public int getWascId() {
528-
return 38;
506+
return 38; // http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse
529507
}
530508

531509
@Override

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,15 @@ public String getReference() {
105105
return Constant.messages.getString(MESSAGE_PREFIX + "refs");
106106
}
107107

108-
private String getError(char c) {
108+
private static String getError(char c) {
109109
return Constant.messages.getString(MESSAGE_PREFIX + "error" + c);
110110
}
111111

112-
/*
113-
* This method is called by the active scanner for each GET and POST parameter for every page
114-
* @see org.parosproxy.paros.core.scanner.AbstractAppParamPlugin#scan(org.parosproxy.paros.network.HttpMessage, java.lang.String, java.lang.String)
115-
*/
116112
@Override
117113
public void scan(HttpMessage msg, String param, String value) {
118114

119115
if (this.isStop()) { // Check if the user stopped things
120-
LOGGER.debug("Scanner {} Stopping.", getName());
116+
LOGGER.debug("Scan rule {} Stopping.", getName());
121117
return; // Stop!
122118
}
123119

@@ -223,7 +219,7 @@ && isPage200(verificationMsg)) {
223219
// errors. It is only
224220
// used if the GNU and generic C compiler check fails to find a vulnerability.
225221
if (this.isStop()) { // Check if the user stopped things
226-
LOGGER.debug("Scanner {} Stopping.", getName());
222+
LOGGER.debug("Scan rule {} Stopping.", getName());
227223
return; // Stop!
228224
}
229225
StringBuilder sb2 = new StringBuilder();
@@ -276,13 +272,11 @@ public Map<String, String> getAlertTags() {
276272

277273
@Override
278274
public int getCweId() {
279-
// The CWE id
280275
return 134;
281276
}
282277

283278
@Override
284279
public int getWascId() {
285-
// The WASC ID
286280
return 6;
287281
}
288282

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public class HeartBleedActiveScanRule extends AbstractHostPlugin
5151
/** the timeout, which is controlled by the Attack Strength */
5252
private int timeoutMs = 0;
5353

54-
/** the logger object */
5554
private static final Logger LOGGER = LogManager.getLogger(HeartBleedActiveScanRule.class);
5655

5756
/** Prefix for internationalized messages used by this rule */
@@ -868,7 +867,6 @@ public class HeartBleedActiveScanRule extends AbstractHostPlugin
868867
0x40,
869868
0x00 // payload length to be sent back by the server. 0x40 0x00 = 16384 in decimal
870869
// Note: No actual payload sent!
871-
// Note: No actual padding sent!
872870
};
873871

874872
@Override

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ private String getEmptyValueResponse(String paramName) throws IOException {
267267
* @param value the value that need to be checked
268268
* @return true if it seems to be encrypted
269269
*/
270-
private boolean isEncrypted(byte[] value) {
270+
private static boolean isEncrypted(byte[] value) {
271271

272272
// Make sure we have a reasonable sized string
273273
// (encrypted strings tend to be long, and short strings tend to break our numbers)

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,7 @@
4949
public class PathTraversalScanRule extends AbstractAppParamPlugin
5050
implements CommonActiveScanRuleInfo {
5151

52-
/*
53-
* Prefix for internationalised messages used by this rule
54-
*/
52+
// Prefix for internationalised messages used by this rule
5553
private static final String MESSAGE_PREFIX = "ascanrules.pathtraversal.";
5654

5755
private static final Map<String, String> ALERT_TAGS =
@@ -608,7 +606,7 @@ private boolean sendAndCheckPayload(
608606
return false;
609607
}
610608

611-
private String getContentsToMatch(HttpMessage message) {
609+
private static String getContentsToMatch(HttpMessage message) {
612610
return message.getResponseHeader().isHtml()
613611
? StringEscapeUtils.unescapeHtml4(message.getResponseBody().toString())
614612
: message.getResponseHeader().toString() + message.getResponseBody().toString();
@@ -700,7 +698,7 @@ public String match(String contents) {
700698
return matchWinDirectories(contents);
701699
}
702700

703-
private String matchNixDirectories(String contents) {
701+
private static String matchNixDirectories(String contents) {
704702
Pattern procPattern =
705703
Pattern.compile("(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
706704
Pattern etcPattern = Pattern.compile("(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
@@ -727,7 +725,7 @@ private String matchNixDirectories(String contents) {
727725
return null;
728726
}
729727

730-
private String matchWinDirectories(String contents) {
728+
private static String matchWinDirectories(String contents) {
731729
if (contents.contains("Windows")
732730
&& Pattern.compile("Program\\sFiles").matcher(contents).find()) {
733731
return "Windows";

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public class RemoteCodeExecutionCve20121823ScanRule extends AbstractAppPlugin
5454
*/
5555
private static final Vulnerability VULN = Vulnerabilities.getDefault().get("wasc_20");
5656

57-
/** the logger object */
5857
private static final Logger LOGGER =
5958
LogManager.getLogger(RemoteCodeExecutionCve20121823ScanRule.class);
6059

0 commit comments

Comments
 (0)