Skip to content
Snippets Groups Projects

Add ['allowed_classes' => FALSE] to unserialize() in migrate modules

Open Benji Fisher requested to merge issue/drupal-3525170:3525170-migrate-unserialize into 11.x
2 unresolved threads

Closes #3525170

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
65 65 if (isset($cached['id'])) {
66 66 // Explicitly unserialize this to create a new object
67 67 // instance.
68 $definitions[$cached['id']] = unserialize($cached['content']);
68 $definitions[$cached['id']] = unserialize($cached['content'], ['allowed_classes' => FALSE]);
  • Comment on lines -68 to +68

    This use of unserialize() seems like the most likely to need classes other than stdClass, since it is parsing annotations from doc blocks. But that also means this is the safest place to use unserialize(): it is being applied to data extracted from PHP classes in the codebase, not from the database.

    Of course, if we can restrict the allowed classes, we should do that, since it is safer if we do not have to worry about it.

  • Please register or sign in to reply
  • 173 173 catch (\Exception) {
    174 174 $result = FALSE;
    175 175 }
    176 return $result !== FALSE ? unserialize($result) : $default;
    176 return $result !== FALSE ? unserialize($result, ['allowed_classes' => ['stdClass']]) : $default;
    • The tests fail if stdClass is not allowed here. Probably it should also be allowed in the Variable and VariableMultiRow source plugins, too. It might even be a good idea to allow stdClass everywhere.

    • Please register or sign in to reply
    Please register or sign in to reply
    Loading