Issue #316 Fixing issues in XSSInImgTagAttribute Vulnerability

src/main/java/org/sasanlabs/service/vulnerability/xss/reflected/UrlParamBasedImgTagAttrInjection.java → src/main/java/org/sasanlabs/service/vulnerability/xss/reflected/XSSInImgTagAttribute.java

@@ -1,7 +1,6 @@
 package org.sasanlabs.service.vulnerability.xss.reflected;
 
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Set;
 import org.apache.commons.text.StringEscapeUtils;
 import org.sasanlabs.internal.utility.LevelConstants;
@@ -14,22 +13,28 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.util.HtmlUtils;
 
 /**
  * This class contains XSS vulnerabilities which are present in Image Tag attribute.
  *
  * @author KSASAN [email protected]
+ * @author jpralle [email protected]
+ * @author t0bel1x [email protected]
+ * @author pdelmonego [email protected]
  */
 @VulnerableAppRestController(descriptionLabel = "XSS_VULNERABILITY", value = "XSSInImgTagAttribute")
-public class UrlParamBasedImgTagAttrInjection {
+public class XSSInImgTagAttribute {
 
     private static final String OWASP_IMAGE = "images/owasp.png";
     private static final String ZAP_IMAGE = "images/ZAP.png";
     private static final String PARAMETER_NAME = "src";
+    public static final String IMAGE_RESOURCE_PATH = "/VulnerableApp/images/";
+    public static final String FILE_EXTENSION = ".png";
 
-    private Set<String> allowedValues = new HashSet<>();
+    private final Set<String> allowedValues = new HashSet<>();
 
-    public UrlParamBasedImgTagAttrInjection() {
+    public XSSInImgTagAttribute() {
         allowedValues.add(OWASP_IMAGE);
         allowedValues.add(ZAP_IMAGE);
     }
@@ -41,13 +46,12 @@ public UrlParamBasedImgTagAttrInjection() {
             description = "XSS_DIRECT_INPUT_SRC_ATTRIBUTE_IMG_TAG")
     @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_1, htmlTemplate = "LEVEL_1/XSS")
     public ResponseEntity<String> getVulnerablePayloadLevel1(
-            @RequestParam Map<String, String> queryParams) {
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
         String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
-        StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            payload.append(String.format(vulnerablePayloadWithPlaceHolder, map.getValue()));
-        }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
+
+        return new ResponseEntity<>(
+                String.format(vulnerablePayloadWithPlaceHolder, imageLocation), HttpStatus.OK);
     }
 
     // Adding Untrusted Data into Src tag between quotes is beneficial but not
@@ -57,13 +61,13 @@ public ResponseEntity<String> getVulnerablePayloadLevel1(
             description = "XSS_QUOTES_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG")
     @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_2, htmlTemplate = "LEVEL_1/XSS")
     public ResponseEntity<String> getVulnerablePayloadLevel2(
-            @RequestParam Map<String, String> queryParams) {
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
         String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";
-        StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            payload.append(String.format(vulnerablePayloadWithPlaceHolder, map.getValue()));
-        }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
+
+        String payload = String.format(vulnerablePayloadWithPlaceHolder, imageLocation);
+
+        return new ResponseEntity<>(payload, HttpStatus.OK);
     }
 
     // Good way for HTML escapes so hacker cannot close the tags but can use event
@@ -73,16 +77,16 @@ public ResponseEntity<String> getVulnerablePayloadLevel2(
             description = "XSS_HTML_ESCAPE_ON_DIRECT_INPUT_SRC_ATTRIBUTE_IMG_TAG")
     @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_3, htmlTemplate = "LEVEL_1/XSS")
     public ResponseEntity<String> getVulnerablePayloadLevel3(
-            @RequestParam Map<String, String> queryParams) {
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
         String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
-        StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            payload.append(
-                    String.format(
-                            vulnerablePayloadWithPlaceHolder,
-                            StringEscapeUtils.escapeHtml4(map.getValue())));
-        }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
+
+        String payload =
+                String.format(
+                        vulnerablePayloadWithPlaceHolder,
+                        StringEscapeUtils.escapeHtml4(imageLocation));
+
+        return new ResponseEntity<>(payload, HttpStatus.OK);
     }
 
     // Good way for HTML escapes so hacker cannot close the tags and also cannot pass brackets but
@@ -94,37 +98,19 @@ public ResponseEntity<String> getVulnerablePayloadLevel3(
                     "XSS_HTML_ESCAPE_ON_DIRECT_INPUT_AND_REMOVAL_OF_VALUES_WITH_PARENTHESIS_SRC_ATTRIBUTE_IMG_TAG")
     @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_4, htmlTemplate = "LEVEL_1/XSS")
     public ResponseEntity<String> getVulnerablePayloadLevel4(
-            @RequestParam Map<String, String> queryParams) {
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
         String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
         StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            if (!map.getValue().contains("(") || !map.getValue().contains(")")) {
-                payload.append(
-                        String.format(
-                                vulnerablePayloadWithPlaceHolder,
-                                StringEscapeUtils.escapeHtml4(map.getValue())));
-            }
-        }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
-    }
 
-    // Good way and can protect against attacks but it is better to have check on
-    // the input values provided if possible.
-    @AttackVector(
-            vulnerabilityExposed = VulnerabilityType.REFLECTED_XSS,
-            description = "XSS_QUOTES_AND_WITH_HTML_ESCAPE_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG")
-    @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_5, htmlTemplate = "LEVEL_1/XSS")
-    public ResponseEntity<String> getVulnerablePayloadLevel5(
-            @RequestParam Map<String, String> queryParams) {
-        String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";
-        StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
+        if (!imageLocation.contains("(") || !imageLocation.contains(")")) {
             payload.append(
                     String.format(
                             vulnerablePayloadWithPlaceHolder,
-                            StringEscapeUtils.escapeHtml4(map.getValue())));
+                            StringEscapeUtils.escapeHtml4(imageLocation)));
         }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
+
+        return new ResponseEntity<>(payload.toString(), HttpStatus.OK);
     }
 
     // Assume here that there is a validator vulnerable to Null Byte which validates the file name
@@ -133,32 +119,61 @@ public ResponseEntity<String> getVulnerablePayloadLevel5(
             vulnerabilityExposed = VulnerabilityType.REFLECTED_XSS,
             description =
                     "XSS_HTML_ESCAPE_PLUS_FILTERING_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG_BUT_NULL_BYTE_VULNERABLE")
-    @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_6, htmlTemplate = "LEVEL_1/XSS")
-    public ResponseEntity<String> getVulnerablePayloadLevel6(
-            @RequestParam Map<String, String> queryParams) {
+    @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_5, htmlTemplate = "LEVEL_1/XSS")
+    public ResponseEntity<String> getVulnerablePayloadLevel5(
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
         String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
         StringBuilder payload = new StringBuilder();
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            String validatedFileName = map.getValue();
-            // Behavior of Null Byte Vulnerable Validator for filename
-            if (map.getValue().contains(Constants.NULL_BYTE_CHARACTER)) {
-                validatedFileName =
-                        map.getValue()
-                                .substring(
-                                        0, map.getValue().indexOf(Constants.NULL_BYTE_CHARACTER));
-            }
-            if (allowedValues.contains(validatedFileName)) {
-                payload.append(
-                        String.format(
-                                vulnerablePayloadWithPlaceHolder,
-                                StringEscapeUtils.escapeHtml4(map.getValue())));
-            }
+
+        String validatedFileName = imageLocation;
+
+        // Behavior of Null Byte Vulnerable Validator for filename
+        if (imageLocation.contains(Constants.NULL_BYTE_CHARACTER)) {
+            validatedFileName =
+                    imageLocation.substring(
+                            0, imageLocation.indexOf(Constants.NULL_BYTE_CHARACTER));
+        }
+
+        if (allowedValues.contains(validatedFileName)) {
+            payload.append(
+                    String.format(
+                            vulnerablePayloadWithPlaceHolder,
+                            StringEscapeUtils.escapeHtml4(imageLocation)));
         }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
+
+        return new ResponseEntity<>(payload.toString(), HttpStatus.OK);
     }
 
     // Good way and can protect against attacks but it is better to have check on
     // the input values provided if possible.
+    @AttackVector(
+            vulnerabilityExposed = VulnerabilityType.REFLECTED_XSS,
+            description = "XSS_QUOTES_AND_WITH_HTML_ESCAPE_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG")
+    @VulnerableAppRequestMapping(
+            value = LevelConstants.LEVEL_6,
+            variant = Variant.SECURE,
+            htmlTemplate = "LEVEL_1/XSS")
+    public ResponseEntity<String> getVulnerablePayloadLevel6(
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
+
+        String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";
+
+        if (allowedValues.contains(imageLocation)) {
+            String payload =
+                String.format(
+                    vulnerablePayloadWithPlaceHolder,
+                    StringEscapeUtils.escapeHtml4(imageLocation));
+
+            return new ResponseEntity<>(payload, HttpStatus.OK);
+        }
+
+        return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
+    }
+
+    // Escape all special characters to their corresponding HTML hex format
+    // and validate input.
+    // Would be even better if Content Security Policy (CSP) is set.
     @AttackVector(
             vulnerabilityExposed = VulnerabilityType.REFLECTED_XSS,
             description =
@@ -168,18 +183,22 @@ public ResponseEntity<String> getVulnerablePayloadLevel6(
             variant = Variant.SECURE,
             htmlTemplate = "LEVEL_1/XSS")
     public ResponseEntity<String> getVulnerablePayloadLevelSecure(
-            @RequestParam Map<String, String> queryParams) {
+            @RequestParam(PARAMETER_NAME) String imageLocation) {
         String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";
-        StringBuilder payload = new StringBuilder();
 
-        for (Map.Entry<String, String> map : queryParams.entrySet()) {
-            if (allowedValues.contains(map.getValue())) {
-                payload.append(
-                        String.format(
-                                vulnerablePayloadWithPlaceHolder,
-                                StringEscapeUtils.escapeHtml4(map.getValue())));
-            }
+        if ((imageLocation.startsWith(IMAGE_RESOURCE_PATH)
+                        && imageLocation.endsWith(FILE_EXTENSION))
+                || allowedValues.contains(imageLocation)) {
+
+            String payload =
+                    String.format(
+                            vulnerablePayloadWithPlaceHolder,
+                            HtmlUtils.htmlEscapeHex(imageLocation));
+
+            return new ResponseEntity<>(payload, HttpStatus.OK);
+
+        } else {
+            return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         }
-        return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
     }
 }

src/test/java/org/sasanlabs/service/vulnerability/xss/reflected/XSSInImgTagAttributeTest.java

@@ -0,0 +1,39 @@
+package org.sasanlabs.service.vulnerability.xss.reflected;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+
+public class XSSInImgTagAttributeTest {
+
+    @ParameterizedTest
+    @CsvSource({
+        "/VulnerableApp/images/ZAP.png,/VulnerableApp/images/ZAP.png",
+        "/VulnerableApp/images/path/to/ZAP.png,/VulnerableApp/images/path/to/ZAP.png",
+        "/VulnerableApp/images/<greek>διαδρομή/προς</greek>/ZAP.png,/VulnerableApp/images/"
+                + "&#x3c;greek&#x3e;&#x3b4;&#x3b9;&#x3b1;&#x3b4;&#x3c1;&#x3bf;&#x3bc;ή/"
+                + "&#x3c0;&#x3c1;&#x3bf;&#x3c2;&#x3c;/greek&#x3e;/ZAP.png"
+    })
+    public void getVulnerablePayloadLevelSecure_validPaths(String input, String expected) {
+        XSSInImgTagAttribute subject = new XSSInImgTagAttribute();
+
+        ResponseEntity<String> actual = subject.getVulnerablePayloadLevelSecure(input);
+
+        String expectedString = "<img src=\"" + expected + "\" width=\"400\" height=\"300\"/>";
+
+        assertEquals(expectedString, actual.getBody());
+    }
+
+    @ParameterizedTest
+    @CsvSource({"''onerror='alert(1);'", " onerror=alert`1`", "http://evil.org/maliciousImage.png"})
+    public void getVulnerablePayloadLevelSecure_exploitsFromLowerLevels(String input) {
+        XSSInImgTagAttribute subject = new XSSInImgTagAttribute();
+
+        ResponseEntity<String> actual = subject.getVulnerablePayloadLevelSecure(input);
+
+        assertSame(actual.getStatusCode(), HttpStatus.BAD_REQUEST);
+    }
+}