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: Refactor sqli scan rule into helper functions #5868

Merged
merged 1 commit into from
Nov 25, 2024
Merged

ascanrules: Refactor sqli scan rule into helper functions #5868

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
190 changes: 110 additions & 80 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 @@ -651,6 +651,96 @@ public void scan(HttpMessage msg, String param, String origParamValue) {
countUnionBasedRequests = 0;
countOrderByBasedRequests = 0;

List<SqlInjectionTestCase> testCases =
List.of(
this::testErrorBasedSqlInjection,
this::testExpressionBasedSqlInjection,
this::testBooleanBasedSqlInjection,
this::testBooleanBasedNoDataSqlInjection,
this::testUnionBasedSqlInjection,
this::testOrderBySqlInjection);

for (SqlInjectionTestCase testCase : testCases) {
if (isStop() || sqlInjectionFoundForUrl) {
break;
}
testCase.run(param, origParamValue);
}

// ###############################

// if a sql injection was found, we should check if the page is flagged as a login page
// in any of the contexts. if it is, raise an "SQL Injection - Authentication Bypass"
// alert in addition to the alerts already raised
if (sqlInjectionFoundForUrl) {
boolean loginUrl = false;

// are we dealing with a login url in any of the contexts?
ExtensionAuthentication extAuth =
(ExtensionAuthentication)
Control.getSingleton()
.getExtensionLoader()
.getExtension(ExtensionAuthentication.NAME);
if (extAuth != null) {
URI requestUri = getBaseMsg().getRequestHeader().getURI();

// using the session, get the list of contexts for the url
List<Context> contextList =
extAuth.getModel()
.getSession()
.getContextsForUrl(requestUri.toString());

// now loop, and see if the url is a login url in each of the contexts in turn..
for (Context context : contextList) {
URI loginUri = extAuth.getLoginRequestURIForContext(context);
if (loginUri != null) {
if (requestUri.getScheme().equals(loginUri.getScheme())
&& requestUri.getHost().equals(loginUri.getHost())
&& requestUri.getPort() == loginUri.getPort()
&& requestUri.getPath().equals(loginUri.getPath())) {
// we got this far.. only the method (GET/POST), user details, query
// params, fragment, and POST params
// are possibly different from the login page.
loginUrl = true;
break;
}
}
}
}
if (loginUrl) {
// raise the alert, using the custom name and description
String vulnname =
Constant.messages.getString(MESSAGE_PREFIX + "authbypass.name");
String vulndesc =
Constant.messages.getString(MESSAGE_PREFIX + "authbypass.desc");

// raise the alert, using the attack string stored earlier for this purpose
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(vulnname)
.setDescription(vulndesc)
.setUri(refreshedmessage.getRequestHeader().getURI().toString())
.setParam(param)
.setAttack(sqlInjectionAttack)
.setMessage(getBaseMsg())
.raise();
} // not a login page
} // no sql Injection Found For Url

} catch (Exception e) {
// Do not try to internationalise this.. we need an error message in any event..
// if it's in English, it's still better than not having it at all.
LOGGER.error("An error occurred checking a url for SQL Injection vulnerabilities", e);
}
}

@FunctionalInterface
interface SqlInjectionTestCase {
void run(String para, String origParamValue) throws IOException;
}

private void testErrorBasedSqlInjection(String param, String origParamValue)
throws IOException {
// Check 1: Check for Error Based SQL Injection (actual error messages).
// for each SQL metacharacter combination to try
for (int sqlErrorStringIndex = 0;
Expand Down Expand Up @@ -773,7 +863,10 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
}
}
}
}

private void testExpressionBasedSqlInjection(String param, String origParamValue)
throws IOException {
// ###############################
// Check 4
// New! I haven't seen this technique documented anywhere else, but it's dead simple.
Expand Down Expand Up @@ -828,8 +921,7 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
// 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";
String modifiedParamValueConfirmForAdd = String.valueOf(paramPlusThree) + "-2";
// Do the attack for ADD variant
expressionBasedAttack(
param,
Expand Down Expand Up @@ -880,7 +972,10 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
// RDBMS specific (ie, it should not live in this scanner)
}
}
}

private void testBooleanBasedSqlInjection(String param, String origParamValue)
throws IOException {
// Check 2: boolean based checks.
// the check goes like so:
// append " and 1 = 1" to the param. Send the query. Check the results. Hopefully they
Expand Down Expand Up @@ -1033,8 +1128,7 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
}
countBooleanBasedRequests++;

String resBodyANDFalseUnstripped =
msg2_and_false.getResponseBody().toString();
String resBodyANDFalseUnstripped = msg2_and_false.getResponseBody().toString();
String resBodyANDFalseStripped =
stripOffOriginalAndAttackParam(
resBodyANDFalseUnstripped,
Expand Down Expand Up @@ -1141,8 +1235,7 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
}
countBooleanBasedRequests++;

String resBodyORTrueUnstripped =
msg2_or_true.getResponseBody().toString();
String resBodyORTrueUnstripped = msg2_or_true.getResponseBody().toString();
String resBodyORTrueStripped =
stripOffOriginalAndAttackParam(
resBodyORTrueUnstripped, origParamValue, orValue);
Expand Down Expand Up @@ -1265,7 +1358,10 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
}
}
}
}

private void testBooleanBasedNoDataSqlInjection(String param, String origParamValue)
throws IOException {
// check 2a: boolean based logic, where the original query returned *no* data. Here we
// append " OR 1=1" in an attempt to extract *more* data
// and then verify the results by attempting to reproduce the original results by
Expand Down Expand Up @@ -1306,8 +1402,7 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
// if the results of the "OR 1=1" exceed the original query (unstripped, by more
// than a 20% size difference, say), we may be onto something.
// TODO: change the percentage difference threshold based on the alert threshold
if ((resBodyORTrueUnstripped.length()
> (mResBodyNormalUnstripped.length() * 1.2))) {
if ((resBodyORTrueUnstripped.length() > (mResBodyNormalUnstripped.length() * 1.2))) {
LOGGER.debug(
"Check 2a, unstripped html output for OR TRUE condition [{}] produced sufficiently larger results than the original message",
sqlBooleanOrTrueValue);
Expand All @@ -1330,9 +1425,7 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
String resBodyANDFalseUnstripped = msg2_and_false.getResponseBody().toString();
String resBodyANDFalseStripped =
stripOffOriginalAndAttackParam(
resBodyANDFalseUnstripped,
origParamValue,
sqlBooleanAndFalseValue);
resBodyANDFalseUnstripped, origParamValue, sqlBooleanAndFalseValue);

// does the "AND 1=2" version produce the same as the original (for
// stripped/unstripped versions)
Expand Down Expand Up @@ -1386,7 +1479,10 @@ && matchBodyPattern(msg1, errorPattern, sb)) {
}
}
}
}

private void testUnionBasedSqlInjection(String param, String origParamValue)
throws IOException {
// Check 3: UNION based
// for each SQL UNION combination to try
for (int sqlUnionStringIndex = 0;
Expand Down Expand Up @@ -1445,7 +1541,9 @@ && checkUnionErrors(
}
}
}
}

private void testOrderBySqlInjection(String param, String origParamValue) throws IOException {
// ###############################

// check for columns used in the "order by" clause of a SQL statement. earlier tests
Expand Down Expand Up @@ -1510,9 +1608,7 @@ && checkUnionErrors(
String modifiedAscendingOutputUnstripped = msg5.getResponseBody().toString();
String modifiedAscendingOutputStripped =
stripOffOriginalAndAttackParam(
modifiedAscendingOutputUnstripped,
origParamValue,
modifiedParamValue);
modifiedAscendingOutputUnstripped, origParamValue, modifiedParamValue);

// set up two little arrays to ease the work of checking the unstripped output, and
// then the stripped output
Expand Down Expand Up @@ -1620,72 +1716,6 @@ && checkUnionErrors(
}
}
}

// ###############################

// if a sql injection was found, we should check if the page is flagged as a login page
// in any of the contexts. if it is, raise an "SQL Injection - Authentication Bypass"
// alert in addition to the alerts already raised
if (sqlInjectionFoundForUrl) {
boolean loginUrl = false;

// are we dealing with a login url in any of the contexts?
ExtensionAuthentication extAuth =
(ExtensionAuthentication)
Control.getSingleton()
.getExtensionLoader()
.getExtension(ExtensionAuthentication.NAME);
if (extAuth != null) {
URI requestUri = getBaseMsg().getRequestHeader().getURI();

// using the session, get the list of contexts for the url
List<Context> contextList =
extAuth.getModel()
.getSession()
.getContextsForUrl(requestUri.toString());

// now loop, and see if the url is a login url in each of the contexts in turn..
for (Context context : contextList) {
URI loginUri = extAuth.getLoginRequestURIForContext(context);
if (loginUri != null) {
if (requestUri.getScheme().equals(loginUri.getScheme())
&& requestUri.getHost().equals(loginUri.getHost())
&& requestUri.getPort() == loginUri.getPort()
&& requestUri.getPath().equals(loginUri.getPath())) {
// we got this far.. only the method (GET/POST), user details, query
// params, fragment, and POST params
// are possibly different from the login page.
loginUrl = true;
break;
}
}
}
}
if (loginUrl) {
// raise the alert, using the custom name and description
String vulnname =
Constant.messages.getString(MESSAGE_PREFIX + "authbypass.name");
String vulndesc =
Constant.messages.getString(MESSAGE_PREFIX + "authbypass.desc");

// raise the alert, using the attack string stored earlier for this purpose
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(vulnname)
.setDescription(vulndesc)
.setUri(refreshedmessage.getRequestHeader().getURI().toString())
.setParam(param)
.setAttack(sqlInjectionAttack)
.setMessage(getBaseMsg())
.raise();
} // not a login page
} // no sql Injection Found For Url

} catch (Exception e) {
// Do not try to internationalise this.. we need an error message in any event..
// if it's in English, it's still better than not having it at all.
LOGGER.error("An error occurred checking a url for SQL Injection vulnerabilities", e);
}
}

private boolean checkSpecificErrors(
Expand Down