Skip to content

Commit

Permalink
Merge pull request #5802 from kingthorin/ssti-fix
Browse files Browse the repository at this point in the history
ascanrules: Address SSTI false positive
  • Loading branch information
psiinon authored Dec 13, 2024
2 parents 3894fe0 + b4c90f8 commit 766b839
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 1 deletion.
3 changes: 3 additions & 0 deletions addOns/ascanrules/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Server Side Template Injection (Blind)
- XML External Entity Attack

### Fixed
- A situation where the Server-Side Template Injection (SSTI) scan rule might result in false positives related to the Go payloads (Issue 8622).

### Added
- Standardized Scan Policy related alert tags on the rule.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,9 @@ private void searchForMathsExecution(
+ DELIMITER
+ "[\\w\\W]*";

if (output.contains(renderResult) && output.matches(regex)) {
if (output.contains(renderResult)
&& output.matches(regex)
&& sstiPayload.engineSpecificCheck(regex, output, renderTest)) {

String attack = getOtherInfo(sink.getLocation(), output);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
*/
package org.zaproxy.zap.extension.ascanrules.ssti;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* This represents the code that is necessary to execute an arithmetic operation in Golang template
* engine and the expected result of the operation.
Expand All @@ -36,4 +39,11 @@ public int getExpectedResult(int number1, int number2) {
String concatenated = String.format("%d%d", number1, number2);
return Integer.parseInt(concatenated);
}

@Override
public boolean engineSpecificCheck(String regex, String output, String renderTest) {
Matcher matcher = Pattern.compile(regex).matcher(output);
matcher.matches();
return !matcher.group(0).contains(renderTest.replaceAll("[^A-Za-z0-9]+", ""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,8 @@ public List<String> getRenderTestAndResult() {

return values;
}

public boolean engineSpecificCheck(String regex, String output, String renderTest) {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,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
Expand Down Expand Up @@ -328,4 +359,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);
}
};
}
}

0 comments on commit 766b839

Please sign in to comment.