Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ascanrules: add threshold to SQLi and Path Traversal #5336

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

ascanrules: add threshold to SQLi and Path Traversal #5336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc14d82
Added threshold - low, for directory browsing detects in PathTravers…
mikhailevtikhov Mar 5, 2024
5b72441
Added threshold - low, for expression based detects in SQLi scan rule
mikhailevtikhov Mar 5, 2024
fc81a35
Gradlew spotlessApply code formatting
mikhailevtikhov Mar 5, 2024
36f6e56
Fixed errors in tests after adding thresholds
mikhailevtikhov Mar 13, 2024
9397943
Checks for windows and nix directories are implemented with different…
mikhailevtikhov Mar 29, 2024
c635627
Everything that is implemented in PR 5387 has been removed
mikhailevtikhov Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion addOns/ascanrules/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ All notable changes to this add-on will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased

### Added
- Threshold LOW is added for:
- Expression based detects in SQLi scan rule

## [63] - 2024-02-12
### Changed
Expand Down
159 changes: 85 additions & 74 deletions ...s/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,97 +813,108 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
// query was last run (could be hours ago)
// so to work around this, simply re-run the query again now at this point.
// Note that we are not counting this request in our max number of requests to be issued
refreshedmessage = getNewMsg();
try {
sendAndReceive(refreshedmessage, false); // do not follow redirects
} catch (SocketException ex) {
LOGGER.debug(
"Caught {} {} when accessing: {}",
ex.getClass().getName(),
ex.getMessage(),
refreshedmessage.getRequestHeader().getURI());
return; // Something went wrong, no point continuing
}
if (getAlertThreshold().equals(AlertThreshold.LOW)) {
refreshedmessage = getNewMsg();
try {
sendAndReceive(refreshedmessage, false); // do not follow redirects
} catch (SocketException ex) {
LOGGER.debug(
"Caught {} {} when accessing: {}",
ex.getClass().getName(),
ex.getMessage(),
refreshedmessage.getRequestHeader().getURI());
return; // Something went wrong, no point continuing
}

// String mResBodyNormal = getBaseMsg().getResponseBody().toString();
mResBodyNormalUnstripped = refreshedmessage.getResponseBody().toString();
mResBodyNormalStripped = this.stripOff(mResBodyNormalUnstripped, origParamValue);
// String mResBodyNormal = getBaseMsg().getResponseBody().toString();
mResBodyNormalUnstripped = refreshedmessage.getResponseBody().toString();
mResBodyNormalStripped = this.stripOff(mResBodyNormalUnstripped, origParamValue);

if (!sqlInjectionFoundForUrl
&& doExpressionBased
&& countExpressionBasedRequests < doExpressionMaxRequests) {
if (!sqlInjectionFoundForUrl
&& doExpressionBased
&& countExpressionBasedRequests < doExpressionMaxRequests) {

// first figure out the type of the parameter..
try {
// is it an integer type?
// ZAP: removed URLDecoding because on Variants
// int paramAsInt =
// Integer.parseInt(SqlInjectionScanRule.getURLDecode(origParamValue));
int paramAsInt = Integer.parseInt(origParamValue);

LOGGER.debug("The parameter value [{}] is of type Integer", origParamValue);
// This check is implemented using two variant PLUS(+) and MULT(*)
// first figure out the type of the parameter..
try {
// PLUS variant check the param value "3-2" gives same result as original
// request and param value "4-2" gives different result if original param
// value is 1
// set the parameter value to a string value like "3-2", if the original
// parameter value was "1"
int paramPlusTwo = Math.addExact(paramAsInt, 2);
String modifiedParamValueForAdd = String.valueOf(paramPlusTwo) + "-2";
// set the parameter value to a string value like "4-2", if the original
// parameter value was "1"
int paramPlusThree = Math.addExact(paramAsInt, 3);
String modifiedParamValueConfirmForAdd =
String.valueOf(paramPlusThree) + "-2";
// Do the attack for ADD variant
expressionBasedAttack(
param,
origParamValue,
modifiedParamValueForAdd,
modifiedParamValueConfirmForAdd);
// bale out if we were asked nicely
if (isStop()) {
LOGGER.debug("Stopping the scan due to a user request");
return;
}
// MULT variant check the param value "2/2" gives same result as original
// request and param value "4/2" gives different result if original param
// value is 1
if (!sqlInjectionFoundForUrl
&& countExpressionBasedRequests < doExpressionMaxRequests) {
// set the parameter value to a string value like "2/2", if the original
// is it an integer type?
// ZAP: removed URLDecoding because on Variants
// int paramAsInt =
// Integer.parseInt(SqlInjectionScanRule.getURLDecode(origParamValue));
int paramAsInt = Integer.parseInt(origParamValue);

LOGGER.debug("The parameter value [{}] is of type Integer", origParamValue);
// This check is implemented using two variant PLUS(+) and MULT(*)
try {
// PLUS variant check the param value "3-2" gives same result as
// original
// request and param value "4-2" gives different result if original
// param
// value is 1
// set the parameter value to a string value like "3-2", if the original
// parameter value was "1"
int paramMultTwo = Math.multiplyExact(paramAsInt, 2);
String modifiedParamValueForMult = String.valueOf(paramMultTwo) + "/2";
// set the parameter value to a string value like "4/2", if the original
int paramPlusTwo = Math.addExact(paramAsInt, 2);
String modifiedParamValueForAdd = String.valueOf(paramPlusTwo) + "-2";
// set the parameter value to a string value like "4-2", if the original
// parameter value was "1"
int paramMultFour = Math.multiplyExact(paramAsInt, 4);
String modifiedParamValueConfirmForMult =
String.valueOf(paramMultFour) + "/2";
// Do the attack for MULT variant
int paramPlusThree = Math.addExact(paramAsInt, 3);
String modifiedParamValueConfirmForAdd =
String.valueOf(paramPlusThree) + "-2";
// Do the attack for ADD variant
expressionBasedAttack(
param,
origParamValue,
modifiedParamValueForMult,
modifiedParamValueConfirmForMult);
modifiedParamValueForAdd,
modifiedParamValueConfirmForAdd);
// bale out if we were asked nicely
if (isStop()) {
LOGGER.debug("Stopping the scan due to a user request");
return;
}
// MULT variant check the param value "2/2" gives same result as
// original
// request and param value "4/2" gives different result if original
// param
// value is 1
if (!sqlInjectionFoundForUrl
&& countExpressionBasedRequests < doExpressionMaxRequests) {
// set the parameter value to a string value like "2/2", if the
// original
// parameter value was "1"
int paramMultTwo = Math.multiplyExact(paramAsInt, 2);
String modifiedParamValueForMult =
String.valueOf(paramMultTwo) + "/2";
// set the parameter value to a string value like "4/2", if the
// original
// parameter value was "1"
int paramMultFour = Math.multiplyExact(paramAsInt, 4);
String modifiedParamValueConfirmForMult =
String.valueOf(paramMultFour) + "/2";
// Do the attack for MULT variant
expressionBasedAttack(
param,
origParamValue,
modifiedParamValueForMult,
modifiedParamValueConfirmForMult);
// bale out if we were asked nicely
if (isStop()) {
LOGGER.debug("Stopping the scan due to a user request");
return;
}
}
} catch (ArithmeticException ex) {
LOGGER.debug(
"Caught {} {}. When performing integer math with the parameter value [{}]",
ex.getClass().getName(),
ex.getMessage(),
origParamValue);
}
} catch (ArithmeticException ex) {
} catch (Exception e) {
LOGGER.debug(
"Caught {} {}. When performing integer math with the parameter value [{}]",
ex.getClass().getName(),
ex.getMessage(),
origParamValue);
"The parameter value [{}] is NOT of type Integer", origParamValue);
// TODO: implement a similar check for string types? This probably needs to
// be
// RDBMS specific (ie, it should not live in this scanner)
}
} catch (Exception e) {
LOGGER.debug("The parameter value [{}] is NOT of type Integer", origParamValue);
// TODO: implement a similar check for string types? This probably needs to be
// RDBMS specific (ie, it should not live in this scanner)
}
}

Expand Down
76 changes: 58 additions & 18 deletions ...ules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRuleUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
import fi.iki.elonen.NanoHTTPD.Response;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.parosproxy.paros.core.scanner.Alert;
import org.parosproxy.paros.core.scanner.Plugin;
import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold;
import org.parosproxy.paros.core.scanner.Plugin.AttackStrength;
import org.zaproxy.addon.commonlib.CommonAlertTag;
import org.zaproxy.zap.model.Tech;
Expand Down Expand Up @@ -164,15 +168,19 @@ void shouldNotTargetNonDbTechs() {
assertThat(targets, is(equalTo(false)));
}

@Test
void shouldAlertIfSumExpressionsAreSuccessful() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldAlertIfSumExpressionsAreSuccessful(AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
new ExpressionBasedHandler("/", param, ExpressionBasedHandler.Expression.SUM));
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
Expand All @@ -187,8 +195,12 @@ void shouldAlertIfSumExpressionsAreSuccessful() throws Exception {
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
}

@Test
void shouldAlertIfSumExpressionsAreSuccessfulAndReflectedInResponse() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldAlertIfSumExpressionsAreSuccessfulAndReflectedInResponse(
AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -202,6 +214,7 @@ protected String getContent(String value) {
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
Expand All @@ -216,8 +229,12 @@ protected String getContent(String value) {
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
}

@Test
void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessful() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessful(AlertThreshold alertThreshold)
throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -226,16 +243,20 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessful() throws Exception
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
assertThat(httpMessagesSent, hasSize(greaterThan(1)));
assertThat(alertsRaised, hasSize(0));
}

@Test
void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessfulAndIsReflectedInResponse()
throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessfulAndIsReflectedInResponse(
AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -248,22 +269,27 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessfulAndIsReflectedInRes
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
assertThat(httpMessagesSent, hasSize(greaterThan(1)));
assertThat(alertsRaised, hasSize(0));
}

@Test
void shouldAlertIfMultExpressionsAreSuccessful() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldAlertIfMultExpressionsAreSuccessful(AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
new ExpressionBasedHandler("/", param, ExpressionBasedHandler.Expression.MULT));
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
Expand All @@ -278,8 +304,12 @@ void shouldAlertIfMultExpressionsAreSuccessful() throws Exception {
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
}

@Test
void shouldAlertIfMultExpressionsAreSuccessfulAndReflectedInResponse() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldAlertIfMultExpressionsAreSuccessfulAndReflectedInResponse(
AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -293,6 +323,7 @@ protected String getContent(String value) {
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
Expand All @@ -307,8 +338,12 @@ protected String getContent(String value) {
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
}

@Test
void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessful() throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessful(AlertThreshold alertThreshold)
throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -317,16 +352,20 @@ void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessful() throws Exceptio
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
assertThat(httpMessagesSent, hasSize(greaterThan(1)));
assertThat(alertsRaised, hasSize(0));
}

@Test
void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessfulAndReflectedInResponse()
throws Exception {
@ParameterizedTest
@EnumSource(
value = Plugin.AlertThreshold.class,
names = {"LOW"})
void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessfulAndReflectedInResponse(
AlertThreshold alertThreshold) throws Exception {
// Given
String param = "id";
nano.addHandler(
Expand All @@ -339,6 +378,7 @@ void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessfulAndReflectedInResp
rule.init(
getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value),
parent);
rule.setAlertThreshold(alertThreshold);
// When
rule.scan();
// Then
Expand Down
Loading