Skip to content

Commit

Permalink
ascanrules: Path Traversal add Other Info for directory match Alerts
Browse files Browse the repository at this point in the history
- CHANGELOG > Added change note.
- Message.properties > Added key/value pair supporting the new Alert
details.
- PathTraversalScanRule > Updated to include Other Info on Alerts when
applicable.
- PathTraversalScanRuleUnitTest > Updated to assert Other Info or lack
thereof where applicable.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
  • Loading branch information
kingthorin committed Oct 17, 2024
1 parent c139f92 commit 851c5c1
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 9 deletions.
1 change: 1 addition & 0 deletions addOns/ascanrules/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
## Unreleased
### Changed
- The XML External Entity Attack scan rule now include example alert functionality for documentation generation purposes (Issue 6119).
- The Path Traversal scan rule now includes further details when directory matches are made (Issue 8379).

### Fixed
- Added more checks for valid .htaccess files to reduce false positives (Issue 7632).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
*/
private static final ContentsMatcher WIN_PATTERN =
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
private static final String WIN_DIR_EVIDENCE = "Windows";
private static final String[] WIN_LOCAL_FILE_TARGETS = {
// Absolute Windows file retrieval (we suppose C:\\)
"c:/Windows/system.ini",
Expand Down Expand Up @@ -116,6 +117,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
// Dot used to match 'x' or '!' (used in AIX)
private static final ContentsMatcher NIX_PATTERN =
new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
private static final String NIX_DIR_EVIDENCE = "etc";
private static final String[] NIX_LOCAL_FILE_TARGETS = {
// Absolute file retrieval
"/etc/passwd",
Expand Down Expand Up @@ -644,12 +646,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {

private AlertBuilder createMatchedAlert(
String param, String attack, String evidence, int check) {
return newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setParam(param)
.setAttack(attack)
.setEvidence(evidence)
.setAlertRef(getId() + "-" + check);
AlertBuilder builder =
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setParam(param)
.setAttack(attack)
.setEvidence(evidence)
.setAlertRef(getId() + "-" + check);
if (List.of(NIX_DIR_EVIDENCE, WIN_DIR_EVIDENCE).contains(evidence)) {
builder.setOtherInfo(
Constant.messages.getString(
MESSAGE_PREFIX + "info",
evidence,
evidence.equals(WIN_DIR_EVIDENCE)
? DirNamesContentsMatcher.WIN_MATCHES.toString()
: DirNamesContentsMatcher.NIX_MATCHES.toString()));
}
return builder;
}

@Override
Expand Down Expand Up @@ -691,6 +704,9 @@ public String match(String contents) {

private static class DirNamesContentsMatcher implements ContentsMatcher {

public static final List<String> NIX_MATCHES =
List.of("proc", NIX_DIR_EVIDENCE, "boot", "tmp", "home");

@Override
public String match(String contents) {
String result = matchNixDirectories(contents);
Expand Down Expand Up @@ -721,16 +737,18 @@ private static String matchNixDirectories(String contents) {
&& bootMatcher.find()
&& tmpMatcher.find()
&& homeMatcher.find()) {
return "etc";
return NIX_DIR_EVIDENCE;
}

return null;
}

public static final List<String> WIN_MATCHES = List.of(WIN_DIR_EVIDENCE, "Program Files");

private static String matchWinDirectories(String contents) {
if (contents.contains("Windows")
if (contents.contains(WIN_DIR_EVIDENCE)
&& Pattern.compile("Program\\sFiles").matcher(contents).find()) {
return "Windows";
return WIN_DIR_EVIDENCE;
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ ascanrules.paddingoracle.soln = Update the affected server software, or modify t
ascanrules.parametertamper.desc = Parameter manipulation caused an error page or Java stack trace to be displayed. This indicated lack of exception handling and potential areas for further exploit.
ascanrules.parametertamper.name = Parameter Tampering
ascanrules.parametertamper.soln = Identify the cause of the error and fix it. Do not trust client side input and enforce a tight check in the server side. Besides, catch the exception properly. Use a generic 500 error page for internal server error.
ascanrules.pathtraversal.info = While the evidence field indicates {0}, the rule actually checked that the response contains matches for all of the following: {1}.

ascanrules.pathtraversal.name = Path Traversal

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasSize;
Expand Down Expand Up @@ -163,6 +164,13 @@ void shouldAlertIfAttackResponseListsWindowsDirectories() throws Exception {
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("c:/")));
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
assertThat(
alertsRaised.get(0).getOtherInfo(),
is(
equalTo(
"While the evidence field indicates Windows, the rule actually "
+ "checked that the response contains matches for all of the "
+ "following: [Windows, Program Files].")));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
}

Expand All @@ -181,6 +189,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectories() throws Exception {
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
assertThat(
alertsRaised.get(0).getOtherInfo(),
is(
equalTo(
"While the evidence field indicates etc, the rule actually "
+ "checked that the response contains matches for all of the "
+ "following: [proc, etc, boot, tmp, home].")));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
}

Expand All @@ -199,6 +214,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectoriesInPlainText() throws Except
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
assertThat(
alertsRaised.get(0).getOtherInfo(),
is(
equalTo(
"While the evidence field indicates etc, the rule actually "
+ "checked that the response contains matches for all of the "
+ "following: [proc, etc, boot, tmp, home].")));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
}

Expand Down Expand Up @@ -272,6 +294,7 @@ void shouldRaiseAlertIfResponseHasPasswdFileContentAndPayloadIsNullByteBased()
// Then
assertThat(alertsRaised, hasSize(1));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-2")));
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
}

@Test
Expand All @@ -288,6 +311,7 @@ void shouldRaiseAlertIfResponseHasSystemINIFileContentAndPayloadIsNullByteBased(
// Then
assertThat(alertsRaised, hasSize(1));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-1")));
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
}

@ParameterizedTest
Expand All @@ -308,6 +332,7 @@ void shouldAlertOnCheckFiveBelowHighThresholdUnderValidConditions(AlertThreshold
assertThat(alertsRaised, hasSize(1));
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-5")));
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
}

@Test
Expand Down

0 comments on commit 851c5c1

Please sign in to comment.