Skip to content

Commit 689c92e

Browse files
bkollmarbkollmar
committed
Added if statements to ensure one sendandreceive is called based on status of redirect
created a work around instead of following a redirect to avoid SSRF fixed formatting
1 parent 1dd48bd commit 689c92e

File tree

2 files changed

+133
-40
lines changed

2 files changed

+133
-40
lines changed

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

Lines changed: 132 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,26 @@ 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+
testMessage.getRequestHeader().setHeader("Follow-Redirects", "false");
356+
try {
357+
sendAndReceive(testMessage, false);
358+
} catch (SocketException ex) {
359+
LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage());
360+
} catch (IOException ex) {
361+
LOGGER.warn(
362+
"SSTI vulnerability check failed for parameter [{}] due to an I/O error",
363+
paramName,
364+
ex);
365+
}
366+
367+
int statusCode = testMessage.getResponseHeader().getStatusCode();
368+
String redirectUrl = testMessage.getResponseHeader().getHeader("Location");
354369

355370
if (fixSyntax) {
356371
codeFixPrefixes = WAYS_TO_FIX_CODE_SYNTAX;
@@ -368,7 +383,6 @@ private void searchForMathsExecution(
368383
if (isStop() || found) {
369384
break;
370385
}
371-
372386
List<String> payloadsAndResults = sstiPayload.getRenderTestAndResult();
373387
List<String> renderExpectedResults =
374388
payloadsAndResults.subList(1, payloadsAndResults.size());
@@ -392,45 +406,124 @@ private void searchForMathsExecution(
392406
try {
393407

394408
HttpMessage newMsg = getNewMsg();
409+
395410
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;
411+
if (!(statusCode >= 300 && statusCode < 400 && redirectUrl != null)) {
412+
sendAndReceive(newMsg, false);
413+
414+
for (SinkPoint sink : sinksToTest) {
415+
416+
String output =
417+
sink.getCurrentStateInString(newMsg, paramName, renderTest);
418+
419+
for (String renderResult : renderExpectedResults) {
420+
// Some rendering tests add html tags so we can not only search for
421+
// the delimiters with the arithmetic result inside. Regex searches
422+
// may be expensive, so first we check if the result exist in the
423+
// response and only then we check if it inside the delimiters and
424+
// was originated by our payload.
425+
String regex =
426+
"[\\w\\W]*"
427+
+ DELIMITER
428+
+ ".*"
429+
+ renderResult
430+
+ ".*"
431+
+ DELIMITER
432+
+ "[\\w\\W]*";
433+
434+
if (output.contains(renderResult)
435+
&& output.matches(regex)
436+
&& sstiPayload.engineSpecificCheck(
437+
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+
}
450+
}
451+
}
452+
}
453+
if (statusCode >= 300 && statusCode < 400 && redirectUrl != null) {
454+
try {
455+
for (TemplateFormat format : TEMPLATE_FORMATS) {
456+
// Construct the SSTI payload
457+
String sstiPayload2 =
458+
"zapSSTI'%s7*7%s'"
459+
.formatted(
460+
format.getStartTag(), format.getEndTag());
461+
462+
// Create a new POST request
463+
HttpMessage postMsg = getNewMsg();
464+
postMsg.getRequestHeader().setMethod("POST");
465+
postMsg.getRequestHeader()
466+
.setHeader(
467+
"Content-Type",
468+
"application/x-www-form-urlencoded");
469+
470+
// Manually set the body to prevent url-encoding
471+
String requestBody = paramName + "=" + sstiPayload2;
472+
postMsg.setRequestBody(requestBody);
473+
postMsg.getRequestHeader()
474+
.setContentLength(postMsg.getRequestBody().length());
475+
476+
sendAndReceive(postMsg, false); // Send the raw POST request
477+
478+
// Now send a GET request to check if SSTI execution occurred
479+
HttpMessage getProfileMsg =
480+
new HttpMessage(postMsg.getRequestHeader().getURI());
481+
getProfileMsg.getRequestHeader().setMethod("GET");
482+
483+
// Preserve authentication/session details
484+
getProfileMsg
485+
.getRequestHeader()
486+
.setHeader(
487+
"User-Agent",
488+
postMsg.getRequestHeader().getHeader("User-Agent"));
489+
getProfileMsg
490+
.getRequestHeader()
491+
.setHeader(
492+
"Cookie",
493+
postMsg.getRequestHeader().getHeader("Cookie"));
494+
getProfileMsg
495+
.getRequestHeader()
496+
.setHeader(
497+
"Referer",
498+
postMsg.getRequestHeader().getURI().toString());
499+
getProfileMsg
500+
.getRequestHeader()
501+
.setHeader(
502+
"Origin", postMsg.getRequestHeader().getHostName());
503+
504+
sendAndReceive(getProfileMsg, false); // Fetch profile page
505+
506+
String responseBody = getProfileMsg.getResponseBody().toString();
507+
508+
if (responseBody.contains(
509+
"zapSSTI'49'")) { // Check if SSTI was executed
510+
createAlert(
511+
newMsg.getRequestHeader().getURI().toString(),
512+
paramName,
513+
renderTest,
514+
sstiPayload2)
515+
.setMessage(newMsg)
516+
.raise();
517+
found = true;
518+
break;
519+
}
431520
}
521+
522+
} catch (IOException e) {
523+
LOGGER.warn("Failed to send SSTI test requests: ", e);
432524
}
433525
}
526+
434527
} catch (SocketException ex) {
435528
LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage());
436529
} 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)