Skip to content

Commit

Permalink
Issue SasanLabs#316 fixed code remarks
Browse files Browse the repository at this point in the history 
- renamed class and test
- corrected levels
- removed path traversal
- changed RequestParam from map to single param
- added input validation to level 7
  • Loading branch information
@t0bel1x
t0bel1x committed Mar 31, 2022
1 parent 938abc4 commit 656ca2b
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 189 deletions.
188 changes: 70 additions & 118 deletions ...mgTagAttributeInjectionVulnerability.java → ...y/xss/reflected/XSSInImgTagAttribute.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package org.sasanlabs.service.vulnerability.xss.reflected;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.sasanlabs.internal.utility.LevelConstants;
import org.sasanlabs.internal.utility.Variant;
Expand All @@ -17,28 +13,27 @@
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]
*/
@VulnerableAppRestController(descriptionLabel = "XSS_VULNERABILITY", value = "XSSInImgTagAttribute")
public class XSSInImgTagAttributeInjectionVulnerability {
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";
private static final List<Character> ALLOWED_SPECIAL_CHARS = new ArrayList<>();
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 XSSInImgTagAttributeInjectionVulnerability() {
public XSSInImgTagAttribute() {
allowedValues.add(OWASP_IMAGE);
allowedValues.add(ZAP_IMAGE);

ALLOWED_SPECIAL_CHARS.add('/');
ALLOWED_SPECIAL_CHARS.add('.');
}

// Just adding User defined input(Untrusted Data) into Src tag is not secure.
Expand All @@ -48,16 +43,12 @@ public XSSInImgTagAttributeInjectionVulnerability() {
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
Expand All @@ -67,16 +58,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()));
}
String payload = String.format(vulnerablePayloadWithPlaceHolder, imageLocation);

return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
return new ResponseEntity<>(payload, HttpStatus.OK);
}

// Good way for HTML escapes so hacker cannot close the tags but can use event
Expand All @@ -86,19 +74,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())));
}
String payload =
String.format(
vulnerablePayloadWithPlaceHolder,
StringEscapeUtils.escapeHtml4(imageLocation));

return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
return new ResponseEntity<>(payload, HttpStatus.OK);
}

// Good way for HTML escapes so hacker cannot close the tags and also cannot pass brackets but
Expand All @@ -110,81 +95,78 @@ 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())));
}
if (!imageLocation.contains("(") || !imageLocation.contains(")")) {
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.
// Assume here that there is a validator vulnerable to Null Byte which validates the file name
// only till null byte
@AttackVector(
vulnerabilityExposed = VulnerabilityType.REFLECTED_XSS,
description = "XSS_QUOTES_AND_WITH_HTML_ESCAPE_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG")
description =
"XSS_HTML_ESCAPE_PLUS_FILTERING_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG_BUT_NULL_BYTE_VULNERABLE")
@VulnerableAppRequestMapping(value = LevelConstants.LEVEL_5, htmlTemplate = "LEVEL_1/XSS")
public ResponseEntity<String> getVulnerablePayloadLevel5(
@RequestParam Map<String, String> queryParams) {
@RequestParam(PARAMETER_NAME) String imageLocation) {

String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";
String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
StringBuilder payload = new StringBuilder();

for (Map.Entry<String, String> map : queryParams.entrySet()) {
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(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
// only till null byte
// 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_HTML_ESCAPE_PLUS_FILTERING_ON_INPUT_SRC_ATTRIBUTE_IMG_TAG_BUT_NULL_BYTE_VULNERABLE")
@VulnerableAppRequestMapping(value = LevelConstants.LEVEL_6, htmlTemplate = "LEVEL_1/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 Map<String, String> queryParams) {
@RequestParam(PARAMETER_NAME) String imageLocation) {

String vulnerablePayloadWithPlaceHolder = "<img src=%s width=\"400\" height=\"300\"/>";
StringBuilder payload = new StringBuilder();
String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>";

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 payload =
String.format(
vulnerablePayloadWithPlaceHolder,
StringEscapeUtils.escapeHtml4(imageLocation));

return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
return new ResponseEntity<>(payload, HttpStatus.OK);
}

// Input validation filters all malicious inputs
// 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 =
Expand All @@ -194,48 +176,18 @@ 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 (isSecure(map.getValue())) {
payload.append(
String.format(
vulnerablePayloadWithPlaceHolder,
StringEscapeUtils.escapeHtml4(map.getValue())));
}
}

return new ResponseEntity<String>(payload.toString(), HttpStatus.OK);
}

private boolean isSecure(String possibleImageSourcePath) {

if (possibleImageSourcePath.startsWith("/")) {
return false;
}

if (possibleImageSourcePath.contains("..")) {
return false;
}

for (char c : possibleImageSourcePath.toCharArray()) {
if (isIllegalCharacter(c)) {
return false;
}
if (imageLocation.startsWith(IMAGE_RESOURCE_PATH)
&& imageLocation.endsWith(FILE_EXTENSION)) {
String payload =
String.format(
vulnerablePayloadWithPlaceHolder,
HtmlUtils.htmlEscapeHex(imageLocation));
return new ResponseEntity<>(payload, HttpStatus.OK);
} else {
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
}

return true;
}

private boolean isIllegalCharacter(char c) {

boolean result = !StringUtils.isAlphanumeric(String.valueOf(c));
result = result && !ALLOWED_SPECIAL_CHARS.contains(c);

return result;
}
}
71 changes: 0 additions & 71 deletions ...s/service/vulnerability/xss/reflected/XSSInImgTagAttributeInjectionVulnerabilityTest.java
View file

This file was deleted.

Loading

0 comments on commit 656ca2b

Please sign in to comment.