Skip to content

Commit f349b88

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 removed getting the Location header
1 parent 1dd48bd commit f349b88

File tree

2 files changed

+132
-40
lines changed

2 files changed

+132
-40
lines changed

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

Lines changed: 131 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,25 @@ 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();
354368

355369
if (fixSyntax) {
356370
codeFixPrefixes = WAYS_TO_FIX_CODE_SYNTAX;
@@ -368,7 +382,6 @@ private void searchForMathsExecution(
368382
if (isStop() || found) {
369383
break;
370384
}
371-
372385
List<String> payloadsAndResults = sstiPayload.getRenderTestAndResult();
373386
List<String> renderExpectedResults =
374387
payloadsAndResults.subList(1, payloadsAndResults.size());
@@ -392,45 +405,124 @@ private void searchForMathsExecution(
392405
try {
393406

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