From f349b88dee152d57d58e443bb73a6195f17608b1 Mon Sep 17 00:00:00 2001 From: bkollmar Date: Thu, 3 Apr 2025 17:21:42 -0400 Subject: [PATCH] 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 --- .../extension/ascanrules/SstiScanRule.java | 170 ++++++++++++++---- .../ascanrules/SstiScanRuleUnitTest.java | 2 +- 2 files changed, 132 insertions(+), 40 deletions(-) diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java index 5cb85f86064..a47f3c373bf 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiScanRule.java @@ -244,7 +244,7 @@ private void efficientScan(HttpMessage msg, String paramName, String value) { } if (hasSuspectBehaviourWithPolyglot(paramName, inputPoint)) { - searchForMathsExecution(paramName, inputPoint, false); + searchForMathsExecution(paramName, msg, inputPoint, false); } } @@ -291,7 +291,7 @@ private void reliableScan(HttpMessage msg, String paramName, String value, boole return; } - searchForMathsExecution(paramName, inputPoint, fixSyntax); + searchForMathsExecution(paramName, msg, inputPoint, fixSyntax); } /** @@ -346,11 +346,25 @@ private boolean hasSuspectBehaviourWithPolyglot(String paramName, InputPoint inp * @param fixSyntax declares if should use several prefixes to fix a possible syntax error */ private void searchForMathsExecution( - String paramName, InputPoint inputPoint, boolean fixSyntax) { + String paramName, HttpMessage msg, InputPoint inputPoint, boolean fixSyntax) { ArrayList sinksToTest = new ArrayList<>(inputPoint.getSinkPoints()); boolean found = false; String[] codeFixPrefixes = {""}; String templateFixingPrefix; + HttpMessage testMessage = msg.cloneRequest(); + testMessage.getRequestHeader().setHeader("Follow-Redirects", "false"); + try { + sendAndReceive(testMessage, false); + } catch (SocketException ex) { + LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage()); + } catch (IOException ex) { + LOGGER.warn( + "SSTI vulnerability check failed for parameter [{}] due to an I/O error", + paramName, + ex); + } + + int statusCode = testMessage.getResponseHeader().getStatusCode(); if (fixSyntax) { codeFixPrefixes = WAYS_TO_FIX_CODE_SYNTAX; @@ -368,7 +382,6 @@ private void searchForMathsExecution( if (isStop() || found) { break; } - List payloadsAndResults = sstiPayload.getRenderTestAndResult(); List renderExpectedResults = payloadsAndResults.subList(1, payloadsAndResults.size()); @@ -392,45 +405,124 @@ private void searchForMathsExecution( try { HttpMessage newMsg = getNewMsg(); + setParameter(newMsg, paramName, renderTest); - sendAndReceive(newMsg, false); - - for (SinkPoint sink : sinksToTest) { - - String output = sink.getCurrentStateInString(newMsg, paramName, renderTest); - - for (String renderResult : renderExpectedResults) { - // Some rendering tests add html tags so we can not only search for - // the delimiters with the arithmetic result inside. Regex searches - // may be expensive, so first we check if the result exist in the - // response and only then we check if it inside the delimiters and - // was originated by our payload. - String regex = - "[\\w\\W]*" - + DELIMITER - + ".*" - + renderResult - + ".*" - + DELIMITER - + "[\\w\\W]*"; - - if (output.contains(renderResult) - && output.matches(regex) - && sstiPayload.engineSpecificCheck(regex, output, renderTest)) { - - String attack = getOtherInfo(sink.getLocation(), output); - - createAlert( - newMsg.getRequestHeader().getURI().toString(), - paramName, - renderTest, - attack) - .setMessage(newMsg) - .raise(); - found = true; + if (!(statusCode >= 300 && statusCode < 400)) { + sendAndReceive(newMsg, false); + + for (SinkPoint sink : sinksToTest) { + + String output = + sink.getCurrentStateInString(newMsg, paramName, renderTest); + + for (String renderResult : renderExpectedResults) { + // Some rendering tests add html tags so we can not only search for + // the delimiters with the arithmetic result inside. Regex searches + // may be expensive, so first we check if the result exist in the + // response and only then we check if it inside the delimiters and + // was originated by our payload. + String regex = + "[\\w\\W]*" + + DELIMITER + + ".*" + + renderResult + + ".*" + + DELIMITER + + "[\\w\\W]*"; + + if (output.contains(renderResult) + && output.matches(regex) + && sstiPayload.engineSpecificCheck( + regex, output, renderTest)) { + + String attack = getOtherInfo(sink.getLocation(), output); + + createAlert( + newMsg.getRequestHeader().getURI().toString(), + paramName, + renderTest, + attack) + .setMessage(newMsg) + .raise(); + found = true; + } + } + } + } + if (statusCode >= 300 && statusCode < 400) { + try { + for (TemplateFormat format : TEMPLATE_FORMATS) { + // Construct the SSTI payload + String sstiPayload2 = + "zapSSTI'%s7*7%s'" + .formatted( + format.getStartTag(), format.getEndTag()); + + // Create a new POST request + HttpMessage postMsg = getNewMsg(); + postMsg.getRequestHeader().setMethod("POST"); + postMsg.getRequestHeader() + .setHeader( + "Content-Type", + "application/x-www-form-urlencoded"); + + // Manually set the body to prevent url-encoding + String requestBody = paramName + "=" + sstiPayload2; + postMsg.setRequestBody(requestBody); + postMsg.getRequestHeader() + .setContentLength(postMsg.getRequestBody().length()); + + sendAndReceive(postMsg, false); // Send the raw POST request + + // Now send a GET request to check if SSTI execution occurred + HttpMessage getProfileMsg = + new HttpMessage(postMsg.getRequestHeader().getURI()); + getProfileMsg.getRequestHeader().setMethod("GET"); + + // Preserve authentication/session details + getProfileMsg + .getRequestHeader() + .setHeader( + "User-Agent", + postMsg.getRequestHeader().getHeader("User-Agent")); + getProfileMsg + .getRequestHeader() + .setHeader( + "Cookie", + postMsg.getRequestHeader().getHeader("Cookie")); + getProfileMsg + .getRequestHeader() + .setHeader( + "Referer", + postMsg.getRequestHeader().getURI().toString()); + getProfileMsg + .getRequestHeader() + .setHeader( + "Origin", postMsg.getRequestHeader().getHostName()); + + sendAndReceive(getProfileMsg, false); // Fetch profile page + + String responseBody = getProfileMsg.getResponseBody().toString(); + + if (responseBody.contains( + "zapSSTI'49'")) { // Check if SSTI was executed + createAlert( + newMsg.getRequestHeader().getURI().toString(), + paramName, + renderTest, + sstiPayload2) + .setMessage(newMsg) + .raise(); + found = true; + break; + } } + + } catch (IOException e) { + LOGGER.warn("Failed to send SSTI test requests: ", e); } } + } catch (SocketException ex) { LOGGER.debug("Caught {} {}", ex.getClass().getName(), ex.getMessage()); } catch (IOException ex) { diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java index 7cb3b03f21a..43335ac5ae1 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiScanRuleUnitTest.java @@ -67,7 +67,7 @@ protected int getRecommendMaxNumberMessagesPerParam(Plugin.AttackStrength streng case LOW: return recommendMax; case MEDIUM: - return recommendMax + 2; + return recommendMax + 3; case HIGH: return recommendMax; case INSANE: