[ss-2015-023] FIX Add option to whitelist NotifyUsers template vars

By default, the CMS Admin editable template for the NotifyUsers
action has access to a large number of fields, including
(for instance) Member#Password. This would allow a malicious
CMS Admin to extract other admin passwords by adding a template
emailing these fields to themselves when other admins trigger
the workflow.

README.md

@@ -33,6 +33,15 @@ extending from `ModelAdmin`. `mysite/_config/config.yml`:
 	    extensions:
 	        - AdvancedWorkflowExtension
 
+We strongly recommend also setting the `NotifyUsersWorkflowAction` configuration parameter `whitelist_template_variables` 
+to true on new projects. This configuration will achieve this:
+
+	:::yml
+	NotifyUsersWorkflowAction:
+	    whitelist_template_variables: true
+
+See the Security section below for more details.
+
 ## Concept
 
 In its most basic sense, workflow means that new content or changes to existing content, need to go through an approval process before they're able to be
@@ -299,6 +308,19 @@ Okay, now we need to join up these actions using transitions, so that users can
 * Create another transition on the "Manager Approval" action and call it "Reject and Cancel", then select "Cancel" as the next action, then select "Save".
 * Select the "Save" button at the bottom of the screen to finalize your workflow, and you're done.
 
+## Security
+
+### `whitelist_template_variables`
+
+The `NotifyUsersWorkflowAction` workflow action has a configuration parameter, `whitelist_template_variables`.
+Currently this variable defaults to false in order to retain backwards compatibility. In a future major release it will
+be changed to default to true.
+
+Setting this configuration variable to true will limit template variables available in the email template sent as part
+of the notify users action to a known-safe whitelist. When it is false, the template may reference any accessible parameter.
+As this template is editable in the CMS, whitelisting these parameters ensures CMS admins can not bypass data access
+restrictions.
+
 ## Contributing
 
 ### Translations

code/actions/NotifyUsersWorkflowAction.php

@@ -8,6 +8,11 @@
  */
 class NotifyUsersWorkflowAction extends WorkflowAction {
 
+	/**
+	 * @var bool Should templates be constrained to just known-safe variables.
+	 */
+	private static $whitelist_template_variables = false;
+
 	private static $db = array(
 		'EmailSubject'			=> 'Varchar(100)',
 		'EmailFrom'				=> 'Varchar(50)',
@@ -59,8 +64,9 @@ public function fieldLabels($relations = true) {
 			'EmailSubject'      => _t('NotifyUsersWorkflowAction.EMAILSUBJECT', 'Email subject'),
 			'EmailFrom'         => _t('NotifyUsersWorkflowAction.EMAILFROM', 'Email from'),
 			'ListingTemplateID' => _t('NotifyUsersWorkflowAction.LISTING_TEMPLATE', 
-				'Listing Template - Items will be the list of all actions in the workflow (synonym to Actions). 
-					Also available will be all properties of the current Workflow Instance'),
+				'Listing Template - Items will be the list of all actions in the workflow (synonym to Actions). '.
+				'Also available will be all properties of the current Workflow Instance'
+			),
 			'EmailTemplate'     => _t('NotifyUsersWorkflowAction.EMAILTEMPLATE', 'Email template'),
 			'FormattingHelp'    => _t('NotifyUsersWorkflowAction.FORMATTINGHELP', 'Formatting Help')
 		));
@@ -73,46 +79,53 @@ public function execute(WorkflowInstance $workflow) {
 			return true;
 		}
 
-		$context   = $this->getContextFields($workflow->getTarget());
-		$member    = $this->getMemberFields();
-		$initiator = $this->getMemberFields($workflow->Initiator());
+		$member = Member::currentUser();
+		$initiator = $workflow->Initiator();
+
+		$contextFields   = $this->getContextFields($workflow->getTarget());
+		$memberFields    = $this->getMemberFields($member);
+		$initiatorFields = $this->getMemberFields($initiator);
+
 		$variables = array();
 
-		foreach($context as $field => $val) $variables["\$Context.$field"] = $val;
-		foreach($member as $field => $val)  $variables["\$Member.$field"] = $val;
-		foreach($initiator as $field => $val)  $variables["\$Initiator.$field"] = $val;
+		foreach($contextFields as $field => $val) $variables["\$Context.$field"] = $val;
+		foreach($memberFields as $field => $val)  $variables["\$Member.$field"] = $val;
+		foreach($initiatorFields as $field => $val)  $variables["\$Initiator.$field"] = $val;
 
 		$pastActions = $workflow->Actions()->sort('Created DESC');
 		$variables["\$CommentHistory"] = $this->customise(array(
 			'PastActions'=>$pastActions,
 			'Now'=>SS_Datetime::now()
 		))->renderWith('CommentHistory');
 
+		$from = str_replace(array_keys($variables), array_values($variables), $this->EmailFrom);
 		$subject = str_replace(array_keys($variables), array_values($variables), $this->EmailSubject);
 
-		$item = $workflow->customise(array(
-			'Items'		=> $workflow->Actions(),
-			'Member'	=> Member::currentUser(),
-			'Context'	=> $workflow->getTarget(),
-			'CommentHistory' => $variables["\$CommentHistory"]
-		));
-
-		if ($this->ListingTemplateID) {
+		if ($this->config()->whitelist_template_variables) {
+			$item = new ArrayData(array(
+				'Initiator' => new ArrayData($initiatorFields),
+				'Member' => new ArrayData($memberFields),
+				'Context' => new ArrayData($contextFields),
+				'CommentHistory' => $variables["\$CommentHistory"]
+			));
+		}
+		else {
 			$item = $workflow->customise(array(
-				'Items'		=> $workflow->Actions(),
-				'Member'	=> Member::currentUser(),
-				'Initiator' => $workflow->Initiator(),
-				'Context'	=> $workflow->getTarget(),
+				'Items' => $workflow->Actions(),
+				'Member' => $member,
+				'Context' => new ArrayData($contextFields),
+				'CommentHistory' => $variables["\$CommentHistory"]
 			));
+		}
 
+		if ($this->ListingTemplateID) {
 			$template = DataObject::get_by_id('ListingTemplate', $this->ListingTemplateID);
 			$view = SSViewer::fromString($template->ItemTemplate);
 		} else {
 			$view = SSViewer::fromString($this->EmailTemplate);			
 		}
 
 		$body = $view->process($item);
-		$from = str_replace(array_keys($variables), array_values($variables), $this->EmailFrom);
 
 		foreach($members as $member) {
 			if($member->Email) {