From b1144e1bbc37d47186aea9e5facea4a264603e44 Mon Sep 17 00:00:00 2001 From: kingthorin Date: Wed, 9 Oct 2024 09:29:39 -0400 Subject: [PATCH] ascanrules: Address SSTI false positive - CHANGELOG > Fix note - SstiScanRule > Adjust logic to prevent False Positives. - SstiScanRuleUnitTest > Updated for adjusted logic. Signed-off-by: kingthorin --- addOns/ascanrules/CHANGELOG.md | 1 + .../extension/ascanrules/SstiScanRule.java | 11 +++- .../ascanrules/SstiScanRuleUnitTest.java | 62 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index 3290b41cf80..f690ab46ffe 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Added more checks for valid .htaccess files to reduce false positives (Issue 7632). +- A situation where the Server-Side Template Injection (SSTI) scan rule might result in false positives related to the Go payloads (Issue 8622). ## [68] - 2024-09-24 ### Changed diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java index de624609cc8..d8939f0a83e 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java @@ -25,6 +25,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.commons.lang3.RandomStringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -396,8 +398,11 @@ private void searchForMathsExecution( + ".*" + DELIMITER + "[\\w\\W]*"; + Matcher matcher = Pattern.compile(regex).matcher(output); - if (output.contains(renderResult) && output.matches(regex)) { + if (output.contains(renderResult) + && matcher.matches() + && not(matcher.group(0), renderTest)) { String attack = getOtherInfo(sink.getLocation(), output); @@ -424,6 +429,10 @@ private void searchForMathsExecution( } } + private static boolean not(String group0, String renderTest) { + return !group0.contains(renderTest.replaceAll("[^A-Za-z0-9]+", "")); + } + private AlertBuilder createAlert(String url, String param, String attack, String otherInfo) { return newAlert() .setConfidence(Alert.CONFIDENCE_HIGH) diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java index 8846bbd76b8..6add41b1063 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java @@ -236,6 +236,37 @@ protected Response serve(IHTTPSession session) { assertThat(alertsRaised.get(0).getConfidence(), equalTo(Alert.CONFIDENCE_HIGH)); } + @Test + void shouldReportGoBasedSsti() throws NullPointerException, IOException { + String test = "/shouldReportGoBasedSsti/"; + // Given + nano.addHandler(createGoHandler(test, true)); + HttpMessage msg = getHttpMessage(test + "?name=test"); + rule.setConfig(new ZapXmlConfiguration()); + rule.init(msg, parent); + // When + rule.scan(); + // Then + assertThat(alertsRaised.size(), equalTo(1)); + assertThat(alertsRaised.get(0).getRisk(), equalTo(Alert.RISK_HIGH)); + assertThat(alertsRaised.get(0).getConfidence(), equalTo(Alert.CONFIDENCE_HIGH)); + } + + @Test + void shouldNotReportGoBasedSstiWhenDirectiveEchoed() throws NullPointerException, IOException { + String test = "/shouldNotReportGoBasedSstiWhenDirectiveEchoed/"; + // Given + nano.addHandler(createGoHandler(test, false)); + HttpMessage msg = getHttpMessage(test + "?name=test"); + rule.setConfig(new ZapXmlConfiguration()); + rule.setAttackStrength(Plugin.AttackStrength.MEDIUM); + rule.init(msg, parent); + // When + rule.scan(); + // Then + assertThat(alertsRaised.size(), equalTo(0)); + } + @Test void shouldReturnExpectedMappings() { // Given / When @@ -321,4 +352,35 @@ private static String getSimpleArithmeticResult(String expression) throw new IllegalArgumentException("invalid template code"); } } + + private NanoServerHandler createGoHandler(String path, boolean stripPrint) { + return new NanoServerHandler(path) { + @Override + protected Response serve(IHTTPSession session) { + String name = getFirstParamValue(session, "name"); + String response; + if (name != null) { + if (!name.contains("print")) { + return newFixedLengthResponse(getHtml("sstiscanrule/NoInput.html")); + } + try { + if (name.contains("print")) { + name = name.replaceAll("[^A-Za-z0-9]+", ""); + name = stripPrint ? name.replace("print", "") : name; + } + name = templateRenderMock("{", "}", name); + response = + getHtml( + "sstiscanrule/Rendered.html", + new String[][] {{"name", name}}); + } catch (IllegalArgumentException e) { + response = getHtml("sstiscanrule/ErrorPage.html"); + } + } else { + response = getHtml("sstiscanrule/NoInput.html"); + } + return newFixedLengthResponse(response); + } + }; + } }