This repository has been archived by the owner on Jan 10, 2019. It is now read-only.
Add XSS information for Goodie instant answers. #39
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add question to the Goodie FAQ section. - Add questions to the developer checklist. - Include links to OWASP as an authoritative source on XSS. - Improve indentation in checklist for "sub-questions." - Slightly change text of extant Goodie FAQ. This includes the two most obvious places to attempt to get developer attention on potential XSS issues. The tutorial itself does not include an example of returning HTML, so I did not want to muddy up those waters.
| (This section is still growing! Know what should go here? Then **please** [contribute to the documentation](https://github.com/duckduckgo/duckduckgo-documentation/blob/master/CONTRIBUTING.md)!) | ||
| ### Can Goodie instant answers include the user's query string? | ||
|
|
||
| They probably shouldn't. There is a lot of potential for [cross-site scripting attacks](https://www.owasp.org/index.php/Cross-site_Scripting_%28XSS%29) when working with user-supplied strings. While the platform attempts to mitigate these issues in pure ASCII responses, HTML responses should **never** include a raw query string. It is safest to return only data which is generated by your Goodie itself. |
There was a problem hiding this comment.
"probably" makes it sound like we're unsure about the situation in general. I think it should say something like:
"Yes, however they must be handled very carefully because there is a lot of potential for cross-site scripting attacks when working with user-supplied strings."
|
@mwmiller made a few suggested changes, otherwise LGTM. |
- Equivocate less in the FAQ answer. - Make doubly sure that they understand which sorts of strings may be dangerous in the developer_checklist. As always, big thanks to @moollaza for proof-reading and editorial assistance.
I really need to make fix my markdown handling and color scheme so I can read this more easily in the editor.. instead of pushing out nonsense like this and catching it on the page.
|
@moollaza With the latest couple of commits, I think your concerns are covered. I presumed that you bolded "unsanitized" in your comment for my benefit, not as something to include in the output text. The "which" clause already sort-of qualifies with which text we are concerned. Regardless, if you want bold, I know how to make that happen. 😁 |
|
LGTM 👍 |
moollaza
added a commit
that referenced
this pull request
Mar 11, 2014
Add XSS information for Goodie instant answers.
| - Did you set `is_unsafe` to true? | ||
|
|
||
| - Can this instant answer return an HTML response? | ||
| - Have you guaranteed that the response does not contain unsanitized user-supplied strings (e.g. the query string) which could lead to [cross-site scripting attacks](https://www.owasp.org/index.php/Cross-site_Scripting_%28XSS%29)? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes the two most obvious places to attempt to get developer
attention on potential XSS issues. The tutorial itself does not include
an example of returning HTML, so I did not want to muddy up those
waters.
[Once this pull request is reviewed and vetted, it will resolve issue #33.]