the emperor is naked

the emperor is naked

by Petr Skoda -
Number of replies: 22
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

I have just posted about the truth that Moodle is vulnerable because teachers can add text with embedded Javascript.

My post got deleted!

Exploiting of this problem is absolutely trivial, as teacher you simply add a script tag that creates XMLHttpRequest object and request any Moodle page that accepts admin action. For demonstration I used admin/roles/admins.php with confirmadd and sesskey parameters.


Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Have you created an MDL?
Average of ratings: -
In reply to Marcus Green

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
There is no point in creating any issue because Martin Dougiamas himself said multiple times that it is a feature, not a bug.

See "MDL-61876 Fixes for $CFG->forceclean and move to experimental" and "MDL-60940 Add ability to force cleaning all user texts"

Unfortunately $CFG->forceclean does not solve anything because there are other ways how teachers may sneak in XSS through uploaded files.

Anyway it is a waste of my time here when my posts get deleted.
Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
"There is no point in creating any issue because Martin Dougiamas himself said multiple times that it is a feature, not a bug."
Have you considered making a security report as described here?
https://moodle.org/mod/page/view.php?id=8722
Average of ratings: Useful (1)
In reply to Marcus Green

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

Marcus, do you really not understand that Moodle HQ developers and Martin Dougiamas believe it is fine to let editing teachers add any Javascript to page content? This is being openly marketed as a feature of Moodle. This was reported as a security issue many times before. I complained about this repeatedly when I worked for Moodle HQ. 

The facts are:

  1. Editing teachers may add Javascript to Moodle content - which allows them to become administrators at any time if they wish to do so
  2. Editing teachers may use any random SCORM package from anywhere on the internet - this allows anybody who created the file to become site administrator



Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
"This is being openly marketed as a feature of Moodle. "
I had not seen this marketing, do you have some links?
Average of ratings: -
In reply to Marcus Green

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
Marcus, did you really not know about this "feature"?

How about these docs for example?

Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
"Marcus, did you really not know about this "feature"?"
I read quite a lot about Moodle and I don't recall it being mentioned as a feature.
I am not especially knowledgeable about web security but I had assumed that anything manipulated by Javascript should be checked on the server. Are you suggesting there is a lack of checking during server processing?
Average of ratings: -
In reply to Marcus Green

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
oh, in general there is no server side processing that prevents XSS attacks in areas that only editing teaches can use, only student contributed content is sanitised
Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Richard Oelmann -
Picture of Core developers Picture of Plugin developers Picture of Testers
One of the first actions I used to take on sites I managed was to turn off trusted content in the first place, but also to remove that capability from all standard roles.
If someone specific needs it, the capability can be added to a custom role and permitted in just the context where it is required.
Maybe the quick win is to remove that trusted content capability by default?

Note: I do realise that only addresses the content element and not the uploaded files such as SCORM.
Average of ratings: Useful (5)
In reply to Petr Skoda

Re: the emperor is naked

by Dan Marsden -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers Picture of Plugins guardians Picture of Testers Picture of Translators
well... you did post an example exploit on a Friday afternoon after all the HQ staff had gone home for the week.... which you knew was going to be controversial wink

but... I do agree it's worth looking at this again - there is also some recent discussion in the tracker in restricted/non-public issues from various folks about the challenges of allowing users to add Javascript - particularly focused around the html block, but I think it's worth a healthy discussion here in the public forums too - I know we have a bunch of clients that find the embedding of Javascript that they write themselves to be extremely useful - so to them it's definitely a feature they want to retain.

What technical solutions would you propose to improve this in Moodle? - do you have elegant solutions to dealing with SCORM (disabling it by default maybe?)
Average of ratings: Useful (3)
In reply to Dan Marsden

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
As far as SCORM goes, the only solutions I see are signing and whitelisting of SCORM packages. You need to somehow define what packages are trusted. I suppose you have access to that other Moodle fork codebase, you can have a look what I did there exactly with new permissions and database table that stores list of trusted SCORM package content hashes. I might have a few more ideas if you are interested to chat.

And I would not define my extremely simple code that adds admins as exploit, teachers are clearly allowed to do that, so let's call it a clever extension of Moodle functionality, ok?
Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Dan Marsden -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers Picture of Plugins guardians Picture of Testers Picture of Translators
yeah - signing and adding an "allow-list" of packages is unfortunately likely to be a no-go in some of our high-use SCORM orgs - we have been encouraging that orgs make better use of H%P instead of creating new SCORM packages and while H5P provides some good benefits I don't think it solves any of the security issues with grading or Javascript.

While your code was trivial - the deletion of that post by a moderator suggests that in fact it's a not a "feature" we really want to encourage which was also one of the points you were trying to communicate.

I do think it's worth you dropping in how you solved it in that "other" LMS - not all people reading the posts here are able to access that code-base, and you're in a better position to summarise that than I am!
Average of ratings: -
In reply to Dan Marsden

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
Well, you can solve the SCORM trust issues even for large sites, likely they have the SCORMs stored somewhere externally, so all you need is a plugin that automatically "allow-lists" the packages when they are imported into Moodle from the safe place. I wish the repository code was better designed which would make this easier.

These days it is not imaginable to download random unsigned program executables from internet and run them on your computer (especially without an antivirus). How is it fine to upload random SCORM from internet to web servers with lots of personal data? Javascript in SCORM can gain admin access, with admin access you can take over the whole server and do whatever you want.

I do insist that there is absolutely nothing wrong in posting Javascript code that adds somebody as admin, even a student learning Javascript coding could do it after just one lecture and a few minutes of googling. Ordinary teachers, admins and even some partners do not understand what XSS is - it is not a theoretical risk, they have to see to believe it.

People here must open their eyes and realise how much they have to trust users with editing teacher roles. I wonder how are they going to explain when some random teacher exports personal data of all users from their site, could that be a GDPR breach?
Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Colin Fraser -
Picture of Documentation writers Picture of Testers
I can understand your frustration Petr, but I have to ask how many teachers would understand the processes and have the coding skills to actually exploit this vulnerability. Relatively few, I would suggest. They are more likely to be the people who would be their school Moodle coaches or admins anyway. While this may be a vulnerability, I have to ask what is the likelihood of risk? I would offer this idea though, how likely is it that a disgruntled staff member would use this as an avenue to derail their school Moodle? The other issue is that you say this is "trivial" but I would suspect for most of us it would be "dark art". I don't think anyone would want to see an example of how to do it in the wild as some idiot is likely to ask, "What would really happen if I did this?" Raising the issue as a security vulnerability is a good point and I suspect Martin and others may have to reconsider their definitions of "features" and "bugs". Given the historical examples of "can I do this" that have become major security issues for everything ICT, that may not take too long at all, hopefully. Also, given the recent changes to many nation's legislation to penalties involved, and who is ultimately responsible, it is probably a good idea to prevent this sort of potential breach anyway.  
Average of ratings: -
In reply to Colin Fraser

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
I agree Colin, thanks.

Moodle is not used only by schools where you have the luxury of trusting all your teachers. It is also used by big corporations, governments, hospitals, police, banks, etc. If there is no setting to make Moodle secure ($CFG->forcecelan is not good enough), then partners will sooner or later lose business because this kind of organisations are not willing to make such risks, well, laws may not allow them to.

Please note I am not saying that unsafe features should not be available in Moodle at all, I am saying there should be option to prevent all unsafe content in uploaded files and all user generated content.

Average of ratings: Useful (1)
In reply to Petr Skoda

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
" I am saying there should be option to prevent all unsafe content in uploaded files and all user generated content."
That sounds like a good idea.
Average of ratings: -
In reply to Colin Fraser

Re: the emperor is naked

by Marcus Green -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
"but I have to ask how many teachers would understand the processes "
Understanding it is not necessary, just knowing about it and having access to an example is all that is required.
Average of ratings: -
In reply to Marcus Green

Re: the emperor is naked

by Colin Fraser -
Picture of Documentation writers Picture of Testers

I think that was a point I was trying to make, in a surreptitious way, without trying to upset Petr, in suggesting that the example he gave was removed for this reason. But likely I messed it up, Marcus... 😃


Average of ratings: -
In reply to Petr Skoda

Re: the emperor is naked

by Michael Hawkins -
Picture of Core developers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Testers
Hi all,

I wanted to touch base briefly to let you know this is on our radar, and we have taken the feedback about revisiting the current approach to "trusted users" on board.

As Dan mentioned, there are already some investigations happening into related areas, so this is a topic we can potentially investigate more broadly. I will reply further with more information and next steps, once we have had an opportunity to finalise some of the details.

Thanks,

Mick Hawkins
Team Lead / Application Security Chapter Lead
Moodle HQ
Average of ratings: Useful (6)
In reply to Petr Skoda

Re: the emperor is naked

by Matt Porritt -
Picture of Core developers Picture of Moodle HQ Picture of Particularly helpful Moodlers

Hi All,

Thanks for your discussion everyone. The team has reviewed the points raised in this post and have begun revisiting related Tracker issues against our security position and current Moodle LMS functionality.

Moodle LMS, like most web based applications, exists in a world where security threats are constantly evolving, as are the toolsets and mitigations that are used to combat these threats. As such we need to constantly evaluate our security practices and decisions that were made in the past against the current climate.

It’s also important to understand that any changes in this space could have an impact on thousands of current courses and millions of students and teachers globally; across many varied organisations. This makes broader consultation on topics like this very important.

In terms of next steps, the team will continue to review the related issues to identify workable improvements that can be done to improve Moodle’s security, while considering user impact.  As part of this there will be a dedicated Tracker issue along with a dedicated forum post around this project. Consultation from the broader Moodle community is encouraged in both of these and once they have been created I will link to them in a further reply to this forum discussion as well. We look forward to working with everyone involved and their contributions.

It is worth restating that when a security issue is identified (or suspected), that the published reporting procedure needs to be followed. The related documentation for this is here: https://moodledev.io/general/development/process/security. As per the disclosure policy from this documentation we ask for potential security issues to not be shared publicly, which is why the original post was deleted. 

All reported issues are reviewed and triaged with priority. Although that documentation does make mention of our Security Submission Form, in cases like this where there are suggestions to review known and currently allowed functionality, the best place to raise these is directly in Tracker. That allows for discussion and avoids our Bugcrowd triagers, who are only able to follow our current brief of in/out of scope vulnerabilities.


Regards,


Average of ratings: -
In reply to Matt Porritt

Re: the emperor is naked

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
Hello Matt,

I do not understand what you are saying exactly. Do you admit that allowing teachers to freely use Javascript is a security bug now? Or do you still believe allowing it is a valuable feature for teachers?

I posted the example code here only because the official Moodle documentation clearly said that it is a feature and all Moodle HQ developers are aware of the fact that teachers can use the Javascript. I do not think it is fair for you to hint that I was wrong to start the discussion in the open here, nothing would have happened if I reported this in the tracker.

And for the record about a year ago I offered to your predecessor in HQ to fix this problem, but they were not interested.
Average of ratings: -