diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index 751ca1828c6..d5949c6417d 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -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 diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRule.java index 6cfa31a8e43..194543720db 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRule.java @@ -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) } } diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRuleUnitTest.java index bd343731e8a..337611063cb 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionScanRuleUnitTest.java @@ -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; @@ -164,8 +168,11 @@ 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( @@ -173,6 +180,7 @@ void shouldAlertIfSumExpressionsAreSuccessful() throws Exception { rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then @@ -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( @@ -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 @@ -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( @@ -226,6 +243,7 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessful() throws Exception rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then @@ -233,9 +251,12 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessful() throws Exception 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( @@ -248,6 +269,7 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessfulAndIsReflectedInRes rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.SUM.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then @@ -255,8 +277,11 @@ void shouldNotAlertIfSumConfirmationExpressionIsNotSuccessfulAndIsReflectedInRes 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( @@ -264,6 +289,7 @@ void shouldAlertIfMultExpressionsAreSuccessful() throws Exception { rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then @@ -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( @@ -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 @@ -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( @@ -317,6 +352,7 @@ void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessful() throws Exceptio rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then @@ -324,9 +360,12 @@ void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessful() throws Exceptio 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( @@ -339,6 +378,7 @@ void shouldNotAlertIfMultConfirmationExpressionIsNotSuccessfulAndReflectedInResp rule.init( getHttpMessage("/?" + param + "=" + ExpressionBasedHandler.Expression.MULT.value), parent); + rule.setAlertThreshold(alertThreshold); // When rule.scan(); // Then