processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

Stored XSS in ProcessWire CMS #741

Closed security-breachlock closed 5 years ago

security-breachlock commented 6 years ago

Affected software: ProcessWire CMS version: 3.0.98

Type of vulnerability: XSS (Stored XSS)

Discovered by: Breachlock

Website: https://www.breachlock.com/

Author: Balvinder Singh

Description: ProcessWire CMS is prone to a Persistent Cross-Site Scripting attack that allows a malicious user to inject HTML or scripts that can access any cookies, session tokens, or other sensitive information retained by your browser and used with that site.

Vulnerable URL: http://localhost/processwire-master/processwire-master/processwire/page/lister/

Vulnerable parameter: Template name

Proof of concept: Step1: Login into the processwire cms.

Step2: URL: http://localhost/processwire-master/processwire-master/processwire/page/lister/ pw

lesaff commented 6 years ago

Here's my question. If you have a login credentials to get in to the admin panel, don't you think you sort of already given authorized access to the backend?

I would be more worried about potential XSS injection from an unauthenticated session, from the front-end. I may be wrong here.

szabeszg commented 6 years ago

"Non-superusers" should definitely not be able perform XSS, however, if this issue only affects superusers – who have "unlimited" control over the website anyway – then it is not that serious.

BTW, this issue post does not clearly explain how to reproduce it. Anyone can figure that out? Does this vulnerability really exist in an unmodified 3.0.98?

jmartsch commented 6 years ago

@security-provensec Could you provide a screencast or show somehow, how you injected HTML or a script?

somatonic commented 6 years ago

Must be hard to run a security website in WP.

teppokoivula commented 6 years ago

To my best understanding the steps described in this report don't allow one to inject JavaScript or any other kind of code to the site. Either there were additional steps that were omitted from this report, or this report could be a false positive.

In either case, could you please explain further how to reproduce this issue?

As explained by @szabeszg earlier, ProcessWire does provide to superusers access to features that other users are not able to access, but even then I wasn't able to reproduce this one. On the other hand if this requires access to PHP code, the issue is extremely limited scope, if not entirely void.

Also, please note that if you indeed do find a vulnerability, disclosing it publicly via the GitHub issue tracker is a bad idea. In such cases the responsible thing would be reaching out to ProcessWire's lead developer Ryan directly via http://processwire.com/contact/.

ryancramerdesign commented 6 years ago

@security-provensec Thanks for the report. The ProcessWire admin is intended as a trusted administrative user environment only, so it's true that you can configure things in a way that won't always be desirable (with power comes responsibility), some safeguards can be disabled, etc. Though what's shown in the screenshot looks like some output isn't getting entity encoded when it should be. So if that originated as input to ProcessWire (as opposed to directly modifying the database) then there's likely something there that isn't getting the necessary encoding during output. So far I cannot duplicate this one though. I think what we're missing here is where the input is coming from that resulted in that output? If it's something that can allow a non-superuser to input and affect other users, and be done without directly manipulating the database, that would be a concern. If that's the case, or if you aren't sure, please provide additional detail at https://processwire.com/contact/ (my email). On the other hand, if it doesn't fit that description, then I think it's okay to provide more detail here because it would definitely be something to fix, but not necessarily a security concern. Either way, thanks for the report.

szabeszg commented 6 years ago

Let me be suspicious a bit. I might be totally wrong but opening this issue could be about:

  1. Shedding a bad light on ProcessWire on purpose.
  2. Building SEO for their website.

What makes me think that? Well, anyone with "security expertise" should be able to provide proper steps to introduce the issue. However, in our case 8 days has passed by without a reply from the person opening this issue.

security-breachlock commented 5 years ago

Hi @szabeszg @ryancramerdesign @teppokoivula @jmartsch @lesaff

It's been a busy schedule for us . We will surely reply you within 2-3 days with the proper POC. I hope you'll understand. Thanks for your Patience.

Thanks

security-breachlock commented 5 years ago

Hi @szabeszg @ryancramerdesign @teppokoivula @jmartsch @lesaff

We are attaching the full proof of concept (document & video). I hope this will clear all of your doubts. I hope you'll reply soon

Thanks BreachLock POC TO PROCESSWIRE.txt bandicam 2018-11-26 22-32-20-691.zip

jmartsch commented 5 years ago

According to the video, it seems that the Title of a page is not sanitized, which should be done. This IS a security problem, but to use it, you have to be logged in and have permission to edit pages.

somatonic commented 5 years ago

@jmartsch this is not the Title of a page, it's the Label of the title field (in template context) you can edit only as a superuser).

teppokoivula commented 5 years ago

That's right, and the real issue is with the "Usage" field in Template Editor (or rather core Lister, as this issue also occurs in Pages > Find): it should encode entities, but currently it doesn't, which can lead to problems.

I wouldn't classify this as a vulnerability, considering the context: as Soma pointed out the Template Editor is a Superuser tool – and as Ryan mentioned earlier, Superusers are by design trusted users. If you have superuser access, you're already in full control of the site, more or less. Superusers are, for an example, typically able to install modules – and since the admin theme itself is a module, that puts them in full control of the admin side and its markup in its entirety :)

That being said, this is definitely a bug that should be fixed. Thanks for the report, and doubly so for taking the time to prepare a detailed video, @security-provensec!

Ping @ryancramerdesign.

matjazpotocnik commented 5 years ago

ProcessLister actually does entity encode the label using $sanitizer->entities1() method. The problem is javascript/jquery that manipulates the table header, removing entity encoded characters and replacing it with an unencoded equivalent. The side effect of this, hmm, feature, is, that we can use html markup on labels :-) And javascript ... So maybe this is not something we should fix in admin. But I suspect Ryan's intention was to prevent this... Ryan, if you want to deal with this, check ProcessPageLister.js from line 222, maybe replacing .text() with .html() would be enough.

security-breachlock commented 5 years ago

Hi @teppokoivula

If you are considering this as a vulnerability. Can we get a CVE ID to track this bug?

Thanks BreachLock

kixe commented 5 years ago

@security-provensec thanks for reporting this issue. This is maybe a bug but not a security vulnerability.

GitHub is indeed useful to place advertising and linking HPs. As far I can see your website is running under WP. Find a small list of WP security vulnerabilities here: https://www.cvedetails.com/vulnerability-list/vendor_id-2337/product_id-4096/ Think about switching to PW. Be part of a great community that cares about security.

ryancramerdesign commented 5 years ago

I've pushed @matjazpotocnik fix for this. Thanks! I agree that this is a bug but not a vulnerability. Only superuser can make that particular edit, and superuser is the same as a root user on unix. Meaning, the ability to do anything without limits is already implied by this access role.

security-breachlock commented 5 years ago

Hi @ryancramerdesign @lesaff @szabeszg @jmartsch @somatonic

Thanks for your time and we are fine with all the CVE thing and closing this issue and also wanna know that whether you find this issue a valid one or not?

Thanks BreachLock