Finding a Security Hole

We were developing our own intranet system when we came across the issue, a security hole.
This intranet system, called Intra is a decoupled Drupal site with Vue.js application as the frontend. We use this system for tracking employees’ in-office time, absences and other custom events such as bonus points.

For this decoupled direction, we decided to use the Drupal built-in REST API since we found it to be the most optimal solution for our needs. If you would like to read more about this topic, you can find another blog post “Corporate Intranet on Decoupled Drupal”.

A Little Background

For a deeper understanding, let’s discuss the absence logic in our system.

The absence system is designed to manage the absence requests of employees. There are two types of requests: automatic and manual. The automatic request, as the name suggests, are automatically accepted by the system. The manual, on the other hand, means that there is a workflow process behind the scenes. For handling workflows, we have decided to use the Drupal core Workflows module as it keeps everything simple.

Let’s take a look at the common scenario: an employee wishes to take holiday leave so they submit an absence request on the Intra site. The request will generate a notification to the HR department for which they will then decide whether to accept it or not. If an approval has been made, the absence status can be set in the Intra backend.

That was the first version of the business logic by design.

Road to the Hole

After some time, we received a feature request that informed us that every employee should be able to cancel their own, already accepted absence request.

As shrewd developers, we began to research the requirements for this new request. First we tried to post modifications to an existing absence request using Postman. One of our tests included a status change which was forbidden by the configuration. There was no active transition set for this event. At this point of the investigation the things turned interesting. To our surprise, the system accepted the status change!

We were confounded as to why this happened. We reasoned that we must have used a wrong user to test with, or we made some altering which by-passed the transition validation. After several attempts and double-checking the entire flow, we realised the test parameters were correct and the test case should not have worked.

We dug even deeper into the issue to find that the Workflows module and the REST API were not working correctly together. The problem laid with the Workflows’ transition validation since it depended on the form validation- specifically the select widget’s validation. The following shows the lack of transition validation in the ModerationStateConstraintValidator’s validate method:

/**
* {@inheritdoc}
*/
public function validate($value, Constraint $constraint) {
/** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
$entity = $value->getEntity();
// Ignore entities that are not subject to moderation anyway.
if (!$this->moderationInformation->isModeratedEntity($entity)) {
return;
}
$workflow = $this->moderationInformation->getWorkflowForEntity($entity);
if (!$workflow->getTypePlugin()->hasState($entity->moderation_state->value)) {
// If the state we are transitioning to doesn't exist, we can't validate
// the transitions for this entity further.
$this->context->addViolation($constraint->invalidStateMessage, [
'%state' => $entity->moderation_state->value,
'%workflow' => $workflow->label(),
]);
return;
}
$new_state = $workflow->getTypePlugin()->getState($entity->moderation_state->value);
$original_state = $this->getOriginalOrInitialState($entity);
// If a new state is being set and there is an existing state, validate
// there is a valid transition between them.
if (!$original_state->canTransitionTo($new_state->id())) {
$this->context->addViolation($constraint->message, [
'%from' => $original_state->label(),
'%to' => $new_state->label(),
]);
}
else {
// If we're sure the transition exists, make sure the user has permission
// to use it.
if (!$this->stateTransitionValidation->isTransitionValid($workflow, $original_state, $new_state, $this->currentUser)) {
$this->context->addViolation($constraint->invalidTransitionAccess, [
'%original_state' => $original_state->label(),
'%new_state' => $new_state->label(),
]);
}
}
}

As you can see it only validates the transition’s existence but misses the validation of the permissions for the said transition. And here is the corrected code:

/**
* {@inheritdoc}
*/
public function validate($value, Constraint $constraint) {
/** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
$entity = $value->getEntity();
// Ignore entities that are not subject to moderation anyway.
if
(!$this->moderationInformation->isModeratedEntity($entity)) {
return;
}
$workflow = $this->moderationInformation->getWorkflowForEntity($entity);
if (!$workflow->getTypePlugin()->hasState($entity->moderation_state->value)) {
// If the state we are transitioning to doesn't exist, we can't validate
// the transitions for this entity further.
$this->context->addViolation($constraint->invalidStateMessage, [
'%state' => $entity->moderation_state->value,
'%workflow' => $workflow->label(),
]);
return;
}
$new_state = $workflow->getTypePlugin()->getState($entity->moderation_state->value);
$original_state = $this->getOriginalOrInitialState($entity);
// If a new state is being set and there is an existing state, validate
// there is a valid transition between them.
if (!$original_state->canTransitionTo($new_state->id())) {
$this->context->addViolation($constraint->message, [
'%from' => $original_state->label(),
'%to' => $new_state->label(),
]);
}
else {
// If we're sure the transition exists, make sure the user has permission
// to use it.
if (!$this->stateTransitionValidation->isTransitionValid($workflow, $original_state, $new_state, $this->currentUser)) {
$this->context->addViolation($constraint->invalidTransitionAccess, [
'%original_state' => $original_state->label(),
'%new_state' => $new_state->label(),
]);
}
}
}

As you can see here not just the existence of the transition is validated but the permissions as well. In the admin pages missing of this validation didn’t do any harm because the Select widget validated the existence of the available options, but if you’ve changed the widget to Text type, then you could write any kind of state ID by hand. This behaviour were risky combined with the REST module or other API providing module.

Reporting

We realised that this flaw can be exploited by even simple users, (if they have access to a Node which uses the Workflows module). This security hole has the potential to give users an access validation by-pass, allowing them to publish articles or other content without any supervision.

We decided to report this issue to the Drupal security team, including a detailed description on how they can replicate this behaviour. Next to it, we also provided a possible solution which they can implement.

Tools in the Shed

To test the custom REST API endpoints we decided to use Postman. We found this application is perfect for our needs; many options can be set, it supports a number of authorisation types out of the box, and it is easy to use. At the end Postman was the tool, which helped us detect the bug.

Out of the Wild

We got out of the wild, when the issue was fixed and merged into Drupal core version 8.6.2 after six months of developing and testing. For further information you can find the security advisories here: SA-CORE-2018-006.