Secure coding patterns in Drupal (8/9)

Hi, I'm Sam

dgo.to/@samuel.mortenson

@mortensonsam

Yell at me later for....

  • SA-CORE-2015-003
  • SA-CORE-2015-004
  • SA-2017-001
  • SA-CORE-2017-002
  • SA-CORE-2017-003
  • SA-CORE-2019-003
  • SA-CORE-2020-004
  • SA-CORE-2020-007
  • SA-CORE-2020-012

Last two years of Core

Threats change as APIs harden

SQL Injection hasn't been seen in core since 2014 (you might remember that one)

What the future may bring

  • Access bypass / indirect object references
  • Server side request forgery
  • Cross-site scripting (never escaping this)
  • Remote code execution (but not where you'd expect)

You came here for code!

Let's do the big three

Cross site scripting

(XSS)

Twig has your back

Every variable rendered normally is automatically filtered 👍

Attributes are also safe, in general

<b {{ attributes.addClass('"onmouseover="alert(1)"') }}>Hello</b>
> <b class="&quot;onload=&quot;alert&quot;">Hello</b>

Not wrapping attributes with double quotes

<b class={{ configurable_class }}>Hello</b>
$variables['configurable_class'] = 'foo onclick=alert(bar)';
> <b class=foo onclick=alert(bar)>Hello</b>

Variables are escaped with the XSS filter, but aren't aware of the context they're used in.

Attribute objects escape the ", &, ', <, and > characters, which is pretty darn safe.

Misusing render arrays

$build = ['#type' => 'html_tag', '#tag' => 'a', 'Hello'];
$build['#attributes']['href'] = $user_input;
$build['#markup'] = $user_input;
$build['#allowed_tags'] = ['script'];

$build['#children'] = $user_input;
$user_input = 'javascript:alert("foo")';
> <a href="javascript:alert(\"foo\")">Hello</a>
$build['#markup'] = t($user_input)

All of these allow <script>...</script>

$build = ['#type' => 'inline_template', '#template' => $user_input];

Not filtering in Javascript

var value = $('input.title').val();
$('.error').html('<p>Invalid title "' + value + '"</p>');
var value = $('input.title').val();
$('.error').text('Invalid title "' + value + '"');
var value = $('input.title').val();
$('.error').html(Drupal.t('<p>Invalid title "@title"</p>', {'@title': value});

Better

Best

General tips

Question complexity

Use Twig wherever possible

If you really need to manually escape user input, use standard methods:

  • check_markup($text, $format_id)
  • \Drupal\Component\Utility\Xss::filter($text)
  • \Drupal\Component\Utility\Html::escape($text)
  • Drupal.t(text) // With @params
  • Drupal.checkPlain(text)

SQL injection

(SQLi)

Just use the abstractions

$uuid = \Drupal::database()
  ->select('node')
  ->fields('node', ['uuid'])
  ->condition('nid', $nid)
  ->execute()
  ->fetchField();

SELECT node.uuid AS uuid
FROM
node node
WHERE nid = :db_condition_placeholder_0

$nids = \Drupal::entityQuery('node')
  ->condition('title', $title)
  ->execute();
SELECT base_table.vid AS vid, base_table.nid AS nid
FROM
node base_table
INNER JOIN node_field_data node_field_data ON node_field_data.nid = base_table.nid
WHERE node_field_data.title LIKE :db_condition_placeholder_0 ESCAPE '\\'

Not using placeholders

\Drupal::database()
  ->query("DELETE FROM people WHERE name = \"$name\"")
  ->execute();
\Drupal::database()
  ->query('DELETE FROM people WHERE name = :name', [
    ':name' => $name,
  ])
  ->execute();
$name = 'myname" OR "" = "';
> DELETE FROM people WHERE name = "myname" OR "" = ""

Using placeholders

Not escaping LIKE

$result = \Drupal::database()
  ->delete('people')
  ->condition('name', '%_' . $name, 'LIKE')
  ->execute();

$name = '%';
> DELETE FROM people WHERE name LIKE "%_%"
$database = \Drupal::database();
$result = $database
  ->delete('people')
  ->condition('name', '%_' . $database->escapeLike($name), 'LIKE')
  ->execute();

Escaping like

Trusting user operators

$result = \Drupal::database()
  ->select('people')
  ->condition('name', $name, $operator)
  ->execute();

$name = 'bar';
$operator = 'IS NOT NULL)
UNION ALL SELECT SID,SSID FROM SESSIONS
JOIN USERS WHERE ("foo" <>';
> SELECT FROM people WHERE (name IS NOT NULL)
UNION ALL SELECT SID,SSID FROM SESSIONS
JOIN USERS WHERE ("foo" <> "bar")

General tips

Use placeholders everywhere

Try not to write queries by hand

Escape conditions when using LIKE

Remote code execution

(RCE)

No golden rule

But you should watch out for the classics

`magick convert $filename output.png`;
shell_exec("magick convert $filename output.png");
unserialize($user_input);
eval("is_null($user_input)");
shell_exec('magick convert ' . escapeshellarg($filename) . ' output.png');
unserialize($user_input, ['allowed_classes' => FALSE]);

RCE can be hard to spot!

{{ content }}
{{ {'#markup': '<b>Render arrays are cool!</b>'} }}
{{ {"#lazy_builder": ["shell_exec", ["touch /tmp/hello"]]} }}

(Mitigated in Drupal 9)

😱

😱

General tips

Watch out for dynamic shell commands

Never, ever, ever unserialize user input

Try not to use eval or any equivalent

Bugs I think we should care about more

Indirect Object Reference

(IDOR)

Check access when referencing things - for example Entity access, permission checks, and owner checks.

Server Side Request Forgery

(SSRF)

Ensure that all URLs are validated before executing an HTTP call.

If you have to let users give you URLs to request, ensure they're https://, and external or better yet proxied.

Untrusted file operations

You may be vulnerable to...a lot.

(ALOT) 😉

Use core's File API whenever possible. If you need to do custom operations, strictly validate user paths.

For example, check the protocol (phar://_, check for path traversal (../../), check extension (.php), check filename (.htaccess)

A parting thought

Make security a part of your work, culture, and code.

 

 

@mortensonsam / Slack @samuel.mortenson / mortenson.coffee

Made with Slides.com