-
Notifications
You must be signed in to change notification settings - Fork 100
XSS Starter #1383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
XSS Starter #1383
Conversation
| // Simple XSS payloads inspired by big-list-of-naughty-strings | ||
| // https://github.com/minimaxir/big-list-of-naughty-strings/blob/master/blns.txt | ||
| val XSS_PAYLOADS = listOf( | ||
| "javascript:alert('XSS')", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these examples are fine, but not the first javascript:alert('XSS'). it can be used in manual testing, but for automated API testing, might lead to too many false positives. remove it and keep the rest
| var payloadInjected = false | ||
|
|
||
| // Attempt to inject payload into parameters | ||
| actionCopy.parameters.filter { it is BodyParam || it is QueryParam || it is PathParam }.forEach { param -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this check necessary? i mean, why skipping HeaderParam? i guess we could skip the filter isn't it?
| actionCopy.parameters.filter { it is BodyParam || it is QueryParam || it is PathParam }.forEach { param -> | ||
|
|
||
| // Skip if PathParam and payload contains "/" (would break URL structure) | ||
| if(param is PathParam && payload.contains("/")){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory, this should be possible, but we need to fix it. leave the comment: TODO this in theory should be fine if properly escape entries in RestPath
| return@forEach | ||
| } | ||
|
|
||
| val genes = param.primaryGene().flatView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use GeneUtils.getAllStringFields. also should check for staticCheckIfImpactPhenotype()
|
|
||
| try { | ||
| // Check if constraints (min-max length) allow the payload | ||
| if(payload.length >= gene.minLength && payload.length <= gene.maxLength){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are duplicating logic here. better to use gene.setFromStringValue and see whether it returns true/false
| getAction.parameters.filter { it is BodyParam || it is QueryParam || it is PathParam }.forEach { param -> | ||
| val genes = param.primaryGene().flatView() | ||
|
|
||
| genes.forEach { gene -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see previous comments
| } | ||
| } catch(e: Exception){ | ||
| // Constraints might not allow the payload | ||
| log.debug("Failed to inject XSS payload into GET ${gene.name}: ${e.message}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warn
|
|
||
| val added = archive.addIfNeeded(evaluatedIndividual) | ||
| evaluatedIndividual.individual.seeMainExecutableActions().forEach { | ||
| println("$action - ${it.verb} ${it.path} -> ${it.auth.name} - ${added}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this a debug log you forgot to remove?
|
|
||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RequestMethod; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine now, but not longer add new tests to spring-rest-openapi-v2. do for -v3, as -v2 will be refactored out into external driver (long story...)
| ] | ||
| ) | ||
| @GetMapping(path = ["/user/{username}"], produces = [MediaType.TEXT_HTML_VALUE]) | ||
| open fun getUserProfile(@PathVariable username: String): String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these examples are fine... but remember XSS in APIs is not only when response are in HTML, but also in JSON (or XML). eg, think about SPA where the HTML is built on browser. add at least one test where response is JSON
No description provided.