Skip to content

Commit da32258

Browse files
bkollmarbkollmar
committed
Added if statements to ensure one sendandreceive is called based on status of redirect
1 parent 1dd48bd commit da32258

File tree

2 files changed

+99
-40
lines changed

2 files changed

+99
-40
lines changed

‎addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java‎

Lines changed: 98 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ private void efficientScan(HttpMessage msg, String paramName, String value) {
244244
}
245245

246246
if (hasSuspectBehaviourWithPolyglot(paramName, inputPoint)) {
247-
searchForMathsExecution(paramName, inputPoint, false);
247+
searchForMathsExecution(paramName, msg, inputPoint, false);
248248
}
249249
}
250250

@@ -291,7 +291,7 @@ private void reliableScan(HttpMessage msg, String paramName, String value, boole
291291
return;
292292
}
293293

294-
searchForMathsExecution(paramName, inputPoint, fixSyntax);
294+
searchForMathsExecution(paramName, msg, inputPoint, fixSyntax);
295295
}
296296

297297
/**
@@ -346,11 +346,28 @@ private boolean hasSuspectBehaviourWithPolyglot(String paramName, InputPoint inp
346346
* @param fixSyntax declares if should use several prefixes to fix a possible syntax error
347347
*/
348348
private void searchForMathsExecution(
349-
String paramName, InputPoint inputPoint, boolean fixSyntax) {
349+
String paramName, HttpMessage msg, InputPoint inputPoint, boolean fixSyntax) {
350350
ArrayList<SinkPoint> sinksToTest = new ArrayList<>(inputPoint.getSinkPoints());
351351
boolean found = false;
352352
String[] codeFixPrefixes = {""};
353353
String templateFixingPrefix;
354+
HttpMessage testMessage = msg.cloneRequest();
355+
356+
// First request - Do not follow redirects
357+
testMessage.getRequestHeader().setHeader("Follow-Redirects", "false");
358+
try {
359+
sendAndReceive(testMessage, false);
360+
} catch (SocketException ex) {
361+
LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage());
362+
} catch (IOException ex) {
363+
LOGGER.warn(
364+
"SSTI vulnerability check failed for parameter [{}] due to an I/O error",
365+
paramName,
366+
ex);
367+
}
368+
369+
int statusCode = testMessage.getResponseHeader().getStatusCode();
370+
String redirectUrl = testMessage.getResponseHeader().getHeader("Location");
354371

355372
if (fixSyntax) {
356373
codeFixPrefixes = WAYS_TO_FIX_CODE_SYNTAX;
@@ -368,7 +385,6 @@ private void searchForMathsExecution(
368385
if (isStop() || found) {
369386
break;
370387
}
371-
372388
List<String> payloadsAndResults = sstiPayload.getRenderTestAndResult();
373389
List<String> renderExpectedResults =
374390
payloadsAndResults.subList(1, payloadsAndResults.size());
@@ -392,45 +408,88 @@ private void searchForMathsExecution(
392408
try {
393409

394410
HttpMessage newMsg = getNewMsg();
411+
395412
setParameter(newMsg, paramName, renderTest);
396-
sendAndReceive(newMsg, false);
397-
398-
for (SinkPoint sink : sinksToTest) {
399-
400-
String output = sink.getCurrentStateInString(newMsg, paramName, renderTest);
401-
402-
for (String renderResult : renderExpectedResults) {
403-
// Some rendering tests add html tags so we can not only search for
404-
// the delimiters with the arithmetic result inside. Regex searches
405-
// may be expensive, so first we check if the result exist in the
406-
// response and only then we check if it inside the delimiters and
407-
// was originated by our payload.
408-
String regex =
409-
"[\\w\\W]*"
410-
+ DELIMITER
411-
+ ".*"
412-
+ renderResult
413-
+ ".*"
414-
+ DELIMITER
415-
+ "[\\w\\W]*";
416-
417-
if (output.contains(renderResult)
418-
&& output.matches(regex)
419-
&& sstiPayload.engineSpecificCheck(regex, output, renderTest)) {
420-
421-
String attack = getOtherInfo(sink.getLocation(), output);
422-
423-
createAlert(
424-
newMsg.getRequestHeader().getURI().toString(),
425-
paramName,
426-
renderTest,
427-
attack)
428-
.setMessage(newMsg)
429-
.raise();
430-
found = true;
413+
if (!(statusCode >= 300 && statusCode < 400 && redirectUrl != null)) {
414+
sendAndReceive(newMsg, false);
415+
416+
for (SinkPoint sink : sinksToTest) {
417+
418+
String output = sink.getCurrentStateInString(newMsg, paramName, renderTest);
419+
420+
for (String renderResult : renderExpectedResults) {
421+
// Some rendering tests add html tags so we can not only search for
422+
// the delimiters with the arithmetic result inside. Regex searches
423+
// may be expensive, so first we check if the result exist in the
424+
// response and only then we check if it inside the delimiters and
425+
// was originated by our payload.
426+
String regex =
427+
"[\\w\\W]*"
428+
+ DELIMITER
429+
+ ".*"
430+
+ renderResult
431+
+ ".*"
432+
+ DELIMITER
433+
+ "[\\w\\W]*";
434+
435+
if (output.contains(renderResult)
436+
&& output.matches(regex)
437+
&& sstiPayload.engineSpecificCheck(regex, output, renderTest)) {
438+
439+
String attack = getOtherInfo(sink.getLocation(), output);
440+
441+
createAlert(
442+
newMsg.getRequestHeader().getURI().toString(),
443+
paramName,
444+
renderTest,
445+
attack)
446+
.setMessage(newMsg)
447+
.raise();
448+
found = true;
449+
}
431450
}
432451
}
433452
}
453+
if (statusCode >= 300 && statusCode < 400 && redirectUrl != null) {
454+
sendAndReceive(newMsg, true);
455+
for (SinkPoint sink : sinksToTest) {
456+
457+
String output = sink.getCurrentStateInString(newMsg, paramName, renderTest);
458+
459+
for (String renderResult : renderExpectedResults) {
460+
// Some rendering tests add html tags so we can not only search for
461+
// the delimiters with the arithmetic result inside. Regex searches
462+
// may be expensive, so first we check if the result exist in the
463+
// response and only then we check if it inside the delimiters and
464+
// was originated by our payload.
465+
String regex =
466+
"[\\w\\W]*"
467+
+ DELIMITER
468+
+ ".*"
469+
+ renderResult
470+
+ ".*"
471+
+ DELIMITER
472+
+ "[\\w\\W]*";
473+
474+
if (output.contains(renderResult)
475+
&& output.matches(regex)
476+
&& sstiPayload.engineSpecificCheck(regex, output, renderTest)) {
477+
478+
String attack = getOtherInfo(sink.getLocation(), output);
479+
480+
createAlert(
481+
newMsg.getRequestHeader().getURI().toString(),
482+
paramName,
483+
renderTest,
484+
attack)
485+
.setMessage(newMsg)
486+
.raise();
487+
found = true;
488+
}
489+
}
490+
}
491+
}
492+
434493
} catch (SocketException ex) {
435494
LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage());
436495
} catch (IOException ex) {

‎addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ protected int getRecommendMaxNumberMessagesPerParam(Plugin.AttackStrength streng
6767
case LOW:
6868
return recommendMax;
6969
case MEDIUM:
70-
return recommendMax + 2;
70+
return recommendMax + 3;
7171
case HIGH:
7272
return recommendMax;
7373
case INSANE:

0 commit comments

Comments
 (0)