Hacker News new | more | comments | ask | show | jobs | submit login
Evil Teacher: Code Injection in Moodle (ripstech.com)
77 points by martinbdz 8 months ago | hide | past | web | favorite | 26 comments

I think the patch should have been to rip out eval() and build/use a shunting-yard parser/evaluator. A quick search found this: https://github.com/ircmaxell/php-math-parser

Leaving a call to eval() with user supplied input, no matter how well filtered, seems like way too big of a risk.

Agreed, when you are on patch #4 because the previous patches weren't effective, and you can't verify patch #4 because of time constraints, it might be time to step back and reconsider the approach.

Sounds like they should just not be eval()ing user input regardless of how much sanitization they throw at it.

IMHO precedence climbing (a refactored version of recursive descent) would be much simpler and makes parsing seem almost trivial, since it can be made table-driven:


Speaking of parsing simplicity and software design in general, you've coincidentally found a great example of how not to (ab)use OOP... one line of actual work wrapped in a dozen lines of boilerplate and replicated across multiple files with tiny variations:


To put things into perspective, a precedence-climber for 13 levels of precedence from C's grammar, including a tokeniser, fits comfortably in <300 LoC of C, the bulk of that being the tokeniser. PHP's higher level nature (especially around string handling) would probably make a corresponding implementation even smaller.

Or use whatever tools are included in their language for safely evaluating strings (like ast.literal_eval in Python).

It's PHP there is none. In fact most languages don't include a safe way to evaluate strings out of the box.

> It's PHP there is none.

This seems to disprove that, no?


Allow me to rephrase, no cheap way to evaluate arbitrary strings without spinning up another vm.

That’s not true either.


That's an extension. It doesn't come with PHP (not "out of the box").

Rails doesn’t come with a default install of Ruby. Xcode doesn’t come pre-installed on macOS. You have to install Chrome on every major desktop OS as it doesn’t come by default.

Installing and configuring your environment to your needs should be entry level stuff for a developer, so what was the point of your reply?


> Can you explain why PHP developers are so uniquely unable to accept honest criticism of their language? Like, whence all the butthurt, bro?

I’m not a PHP developer. Nice try to deflect though! You obvious have an agenda you are trying to push at all costs and don’t want to learn, so have a great day!

This is hilarious because i'm seeing Moodle installations around the UK that are 4 years old with no patches.

My partner works in higher education and they give her blank stares when she mentions how out of date they are. It's a culmination of "Not my problem" and "It works so why fix it?"

The other problem is that Moodle can be a nightmare to upgrade, especially on older installations.

At least in the olden days, the DB migrations were not super solid, so you'd run into issues later due to that. But worse, it seems like every school has a few pet plugins that are absolute garbage but they have to have - and either aren't maintained, or have horribly broken upgrades.

Moodle used to be provided for free as inclusion of the IT services provided by the local authorities. Local authorities stopped running schools (academy) and LA dropped moodle for free so school saw a free service turn into £3k+/year maintenance contract.

And most schools started to get unhappy with moodle because it was hardly updated. IIRC they left the free moodle running at one major version and offered the service contract on the next major version up as a carrot.

Interesting, thank you.

I did look at putting up on my partners domain just so she's got an instance that doesn't rely on a client (she works for Universities). Last time I looked it was all "dump this .tar.gz php stuff into a directory", it was like going back ten years. No standard containers, debs, rpms, anything. Maybe I didn't look hard enough...

I think the only reason moodle i still around is as above (it was free) and some resources are published as packs for it, which are re-design archives of php and files that upload to a directory. I wasn't particularly impressed with it when I managed one.

Drive + classroom is free as it the MS answer, I don't see moodle as a wise investment in time.

> Moodle allows teachers to set up a quiz with many types of questions. Among them is the calculated question which allows teachers to enter a mathematical formula which will be evaluated by Moodle dynamically on randomized input variables.

But... why? What's the practical use for such a quiz?

It says later in the article, it's so that students can't easily share answers.

Would be handy for a ctf competition maybe if I could run a validator against a string. Especially if I wanted to check if some string validates against my program, and there are possibly many flags in the solution space.

If I'm understand you correctly, what you're asking is an undecidable problem.

Moodle is also that piece of software which allows teachers to include arbitrary javascript in the course content, allowing them to XSS themselves to an admin role.

Moodle may be terrible, but the alternatives are much worse. And omg isn’t PHP a mess. It’s like perl but if you keep all the bad bits and throw out the good bits.

Not related to the content of the article but the justified text in the paragraphs makes it harder to read

This is one of the better write up formats I have seen. Good work Robin.

I remember using Moodle in middle school. I was a bit of a script kiddie and tried to break it. It had a WYSIWYG editor for responses (obviously not helpful for breaking in), but you could also get at an HTML editor. I put some JS in the HTML editor and saved the response, but the code did not execute when I viewed the page after saving it. But then when I brought up the editor to remove my tampering, the JS executed. It didn't execute on viewing the page, only on attempting to edit it.

I never came up with anything to use that for (thankfully, since I would have gotten caught and punished). It required the person viewing to try to edit my post (which teachers could do but rarely did) and then bring up the hidden HTML editor. But it was the first time I ever found a security vulnerability and I remember it fondly.

This was over ten years ago. I hope it's been patched.

Applications are open for YC Summer 2019

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact