XSS Security Implementation and Ongoing SQL Injection Hardening#481
XSS Security Implementation and Ongoing SQL Injection Hardening#481LyesRocker wants to merge 5 commits intoSasanLabs:masterfrom
Conversation
Adding 4 Secure ways of handling XSS Reflected
| @@ -0,0 +1,108 @@ | |||
| package org.sasanlabs.service.vulnerability.sampleVulnerability; | |||
There was a problem hiding this comment.
can you please remove SampleVulnerability related classes. Add them to .gitignore as well.
| queryParams, | ||
| LevelConstants.LEVEL_8, | ||
| post -> StringEscapeUtils.escapeHtml4( | ||
| post.replaceAll("(?i)<script.*?>.*?</script>", "") |
There was a problem hiding this comment.
With escapeHtml4., do we even need escaping of script tag?
There was a problem hiding this comment.
I mean there is some flaws i found, for example you can trigger an xss if the charset is explicitly set to UTF-7 you can use a payload like this one :
+ADw-script+AD4-alert(document.location)+ADw-/script+AD4-
Let me think what do you think
| @RequestParam Map<String, String> queryParams) { | ||
| Function<String, String> function = | ||
| (post) -> { | ||
| String sanitizedPost = post.replaceAll("<.*?>", ""); // Delete all the html balises |
There was a problem hiding this comment.
with html4 escape, do we need replaceall?
There was a problem hiding this comment.
Yeah you are right, we dont actually need that replace.
| description = "XSS_HASH_VALIDATION") | ||
| @VulnerableAppRequestMapping( | ||
| value = LevelConstants.LEVEL_10, | ||
| variant = Variant.SECURE, |
There was a problem hiding this comment.
is it possible to add insecure level for this?
| htmlTemplate = "LEVEL_1/XSS") | ||
| public ResponseEntity<String> getVulnerablePayloadLevelSecure4(@RequestParam(PARAMETER_NAME) String imageLocation) { | ||
| String hashedValue = hashSHA256(imageLocation); | ||
| if (hashedValue.equals("bd473875115034776f0fed141a0b6f8cbd46989e0ff1d52864f88a4e48882c75") || |
There was a problem hiding this comment.
is this hashed values of zap image or owasp image? if so can you compute it on runtime. Why because it will help if we update the images, we don't need to change code.
There was a problem hiding this comment.
Yes, we can do that but the ressources requested needs to be defined before running the function.
| description = "XSS_MIME_TYPE_VALIDATION") | ||
| @VulnerableAppRequestMapping( | ||
| value = LevelConstants.LEVEL_9, | ||
| variant = Variant.SECURE, |
There was a problem hiding this comment.
are we sure that this cannot be broken?
There was a problem hiding this comment.
Requesting a MIME type in the backend cannot be bypassed. If the user submits anything other than an image, the requested resource does not appear.
In the other hand if the user submit something like a file, yes the mime type can be bypassed.
| htmlTemplate = "LEVEL_1/XSS") | ||
| public ResponseEntity<String> getVulnerablePayloadLevelSecure2(@RequestParam(PARAMETER_NAME) String imageLocation) { | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.add("Content-Security-Policy", "default-src 'self'; img-src 'self'"); |
There was a problem hiding this comment.
Can you please add a level which is insecure where csp header is lenient if already that level is not there.
There was a problem hiding this comment.
Ok, I'll add an insecure level with a weak CSP.
| mimeType = URLConnection.guessContentTypeFromName(file.getName()); | ||
| } | ||
| if (mimeType != null && mimeType.startsWith("image/")) { | ||
| String vulnerablePayloadWithPlaceHolder = "<img src=\"%s\" width=\"400\" height=\"300\"/>"; |
There was a problem hiding this comment.
better to create a string constant for this line as it is being used multiple times in all the vulnerability levels.
| return new ResponseEntity<>("Invalid Image Hash", HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| private String hashSHA256(String input) { |
There was a problem hiding this comment.
Move it to utils file so that we can utilise it at other places.
| } | ||
| } | ||
|
|
||
| private String bytesToHex(byte[] bytes) { |
There was a problem hiding this comment.
isn't there any utility already whcih does this for us?
| @@ -0,0 +1,23 @@ | |||
| function addingEventListenerToFetchData() { | |||
There was a problem hiding this comment.
please remove all sampleVulnerabilitu related files.
|
Just following up on this @LyesRocker |
2 participants
Hi @preetkaran20,
I hope you're doing well. I’ve implemented secure functionalities for both stored and reflected XSS:
Reflected XSS:
Level 8: Using CSP and escaping HTML entities
Level 9: Using MIME type
Level 10: Using hashed paths
Stored XSS:
Level 8: Escaping HTML and removing all script and img tags
Level 9: Using a regex to remove all HTML tags
Level 10: Escaping HTML and removing all JS calls, e.g., javascript:alert(1);
Let me know your thoughts. If everything is fine, I’ve already started working on the SQL Union injection.
In the meantime, I am documenting each level I secure or don't secure, and I’ll be providing a document on how to escape each security vulnerability (if not secured), to help users understand the application.
Thanks,
Lyes