How to write insecure Drupal 8 code

Hi, I'm Sam

dgo.to/@samuel.mortenson

@DrupalSAM

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

Drupal is secure out of the box

*You can still mess stuff up

*

So you want to render HTML...

Are you rendering user input?

You may be vulnerable to cross site scripting.

(XSS)

1. Find user input

<script>
    document.write('<math><!--');
 </script>
 <i name="--><head><script>//">
alert(1)
<!--</script>-->

2. Craft clever exploit

3. Execute code

4. Celebrate!

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>

How to mess it all up

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)
  • Drupal.checkPlain(text)

So you want to query the database...

Are you querying based on user input?

You may be vulnerable to SQL injection.

(SQLi)

1. Find vulnerable query

"; DROP TABLE users;

" OR 1=1;

2. Craft clever exploit

3. Execute query, leading to data loss, information disclosure, access bypass, etc.

4. Celebrate!

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 '\\'

Sidebar: SQL is crazy

node_load(1);
SELECT revision.vid AS vid, revision.langcode AS langcode, revision.revision_timestamp AS revision_timestamp, revision.revision_uid AS revision_uid, revision.revision_log AS revision_log, base.nid AS nid, base.type AS type, base.uuid AS uuid, CASE base.vid WHEN revision.vid THEN 1 ELSE 0 END AS isDefaultRevision
FROM
node base
INNER JOIN node_revision revision ON revision.vid = base.vid
WHERE base.nid IN (:db_condition_placeholder_0)SELECT revision.*
FROM
node_field_revision revision
WHERE (revision.nid IN (:db_condition_placeholder_0)) AND (revision.vid IN (:db_condition_placeholder_1))
ORDER BY revision.nid ASCSELECT t.*
FROM
node__body t
WHERE (entity_id IN (:db_condition_placeholder_0)) AND (deleted = :db_condition_placeholder_1) AND (langcode IN (:db_condition_placeholder_2, :db_condition_placeholder_3, :db_condition_placeholder_4, :db_condition_placeholder_5, :db_condition_placeholder_6))
ORDER BY delta ASCSELECT t.*
FROM
node__field_meta_tags t
WHERE (entity_id IN (:db_condition_placeholder_0)) AND (deleted = :db_condition_placeholder_1) AND (langcode IN (:db_condition_placeholder_2, :db_condition_placeholder_3, :db_condition_placeholder_4, :db_condition_placeholder_5, :db_condition_placeholder_6))
ORDER BY delta ASCSELECT t.*
FROM
node__panelizer t
WHERE (entity_id IN (:db_condition_placeholder_0)) AND (deleted = :db_condition_placeholder_1) AND (langcode IN (:db_condition_placeholder_2, :db_condition_placeholder_3, :db_condition_placeholder_4, :db_condition_placeholder_5, :db_condition_placeholder_6))
ORDER BY delta ASCSELECT ces.*
FROM
comment_entity_statistics ces
WHERE (ces.entity_id IN (:db_condition_placeholder_0)) AND (ces.entity_type = :db_condition_placeholder_1)⏎

How to mess it all up

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

So you want to write some code...

Are you executing based on user input?

You may be vulnerable to Remote Code Execution.

(RCE)

1. Find user input

POST '/provision-environment/dev";\
echo "'.$key.'" >> ~/.ssh/authorized_keys'

2. Craft clever exploit

3. Execute code

4. Celebrate!

$ ./provision-env.sh "dev";\
echo "ssh-rsa ..." >> ~/.ssh/authorized_keys

No golden rule

But you should watch out for the classics

`magick convert $filename output.png`;
shell_exec("magick convert $filename output.png");
preg_replace($user_regex, $user_replacement, $user_subject);
preg_replace('/.*/e', 'system("whoami")', "hacked");

( PHP < 7 )

unserialize($user_input);
eval("is_null($user_input)");
shell_exec('magick convert ' . escapeshellarg($filename) . ' output.png');
preg_replace('/' . preg_quote($user_regex) . '/i', $user_replacement, $user_subject);

RCE can be hard to spot!

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

https://www.drupal.org/node/2860607

😱

😱

General tips

Watch out for dynamic shell commands

Never, ever, ever unserialize user input

Try not to use eval or any equivalent

The future of Drupal exploits is headless

Headless Drupal relies mostly on entity validation

Form validation

User input

Route access

Entity validation

User input

Entity validation

Traditional Drupal

Headless Drupal with REST

Resource access

Serializer validation

Headless implementations are not mature

How do you render formatted text in a headless app?

How much validation should an app do before sending data to Drupal?

How should credentials be stored in a headless app?

A parting thought

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

Made with Slides.com