fbpx Let's say NO to unsanitised inputs | ComputerMinds Skip to main content

Let's say NO to unsanitised inputs

An article from ComputerMinds - Building with Drupal in the UK since 2005.
7th Mar 2019

Nathan Page

Developer

Last night saw the popular EU Cookie Compliance module fall from grace, as the Drupal community discovered that numerous inputs in the admin form were not being sanitised.

To me, this shows some serious failings in how our community is handling security awareness. Let's do some fixing :)

1) We need to make this OBVIOUS, with clear examples

One of the most important things when trying to get people to write secure code is making them aware of the issues. We need Drupalers of all levels of experience to know and understand the risks posed by unsanitised input, where they come up and how to fix / avoid them.

I did a little internet searching, and found that there's actually a great guide to writing Drupal modules on Drupal.org. It covers a whole bunch of things, and is compiled really nicely.

I noticed that it says how to do forms, but it manages to NOT mention security anywhere. This should be a key thought right now, no? There is a guide to handling text securely, but it's just there and isn't really linked to.

Similarly, the page of Drupal 7 sanitize functions is easily findable, but only if you know to look for it in the first place

Guys and girls, if we're going to help our Drupalers to write secure code we simply have to make it obvious. We shouldn't be assuming young new Drupalers will think to browse around the internet looking for sanitization functions. We shouldn't be assuming they know all the niggly bits that present security issues. We shouldn't be assuming that anyone really know when to use tokens in URLs and when not to. We should be putting all these things right there, saying hey! don't forget to use these! here's why!. We should have articles and guides for writing forms that take the time to cover how to handle the security side of things.

In that vein, surely the Form API reference should surely have a reminder link? A little sidebar with some links to all these guides and articles on writing secure code?

I'm going to go start some conversations and some edits - Drupal documentation is maintained by us, the Drupalers, after all.
Who else out there wants to help move things in the right direction? :)

Update: Security in Drupal 7 - Handle text in a secure fashion is looking a good bit better. Input still welcome though!

2) We need to be aware of what we're installing

81,086 modules report use of the EU Cookie Compliance module. That's a whole bunch of blind installs! Nobody thought to check through the code? Nobody missed the lack of check_plain?

Well, you don't, do you? It's far too easy to assume that things are just fine. Our wonderful Open Source world, protected by our numbers, means that code is safe because it has a thousand people keeping eyes on it. Unless, of course, we're all assuming that somebody else is looking. In that case, as evident here, nobody really takes responsibility - and that's why we end up with module maintainers burning out trying to fight battles alone. In the presence of other people who we know could also do something, humans are significantly less likely to take responsibility.

I've said this before in my previous article discussing security risks to Drupal as we mature - if we took a little more of a moment to check through the modules that we install, we might catch a whole bunch of missed bugs!

I must make explicit that this call isn't just to the big bods and the experienced Drupalers. This task is for you, too, freelancers and small Drupal shops. We all have unique perspectives and unique opportunities that will allow us to see what others have missed - but if nobody is looking then nobody will see anything.

3) Contrib security reviews need help

Unless we're going to go through every module by hand, we need to think about writing some tool to do a basic sanity check on our contrib modules. How hard can it be to see if there's even one instance of a check_plain in a .admin.inc file?

It's admirable and encouraging to see the Drupal Security Team making huge progress on really key modules. Well done guys :) But, as far as I can guess, they're going through modules by hand, line by line. What other way is there?

If I had £50k going spare, I'd put a huge bounty out for anyone that can write an automated tool for spotting missing check_plains. Alas, I really don't have that! But I reckon there must be a decent tool for at least getting a start?

If we can solve this problem for contrib, then we can also solve it for every site's custom modules. And that will be of huge security benefit for Drupalers worldwide.

Huge publicity awaits whoever solves this problem, I'm sure.
Inventors and innovators in the Drupal world, this is your moment!