Token protect download/view actions.

This is as much a security risk as a weak password, since an attacker has to know the mailbox/UID of the message. But doesn't hurt to protect.
horde · Oct 29, 2013 · dbb0c9c · dbb0c9c
1 parent 6e552fe
commit dbb0c9c
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 7 deletions.
diff --git a/imp/lib/Ajax/Application/ShowMessage.php b/imp/lib/Ajax/Application/ShowMessage.php
@@ -335,7 +335,13 @@ public function showMessage($args)
             $result['atc_list'] = $partlist;
         }
 
-        $result['save_as'] = $registry->downloadUrl(htmlspecialchars_decode($result['subject']), array_merge(array('actionID' => 'save_message'), $mbox->urlParams($uid)));
+        $result['save_as'] = IMP_Contents_View::downloadUrl(
+            htmlspecialchars_decode($result['subject']),
+            array_merge(
+                array('actionID' => 'save_message'),
+                $mbox->urlParams($uid)
+            )
+        );
 
         if ($preview) {
             /* Need to grab cached inline scripts. */

diff --git a/imp/lib/Application.php b/imp/lib/Application.php
@@ -509,10 +509,12 @@ public function download(Horde_Variables $vars)
         switch ($vars->actionID) {
         case 'download_all':
             $view_ob = new IMP_Contents_View(new IMP_Indices_Mailbox($vars));
+            $view_ob->checkToken($vars);
             return $view_ob->downloadAll();
 
         case 'download_attach':
             $view_ob = new IMP_Contents_View(new IMP_Indices_Mailbox($vars));
+            $view_ob->checkToken($vars);
             return $view_ob->downloadAttach($vars->id, $vars->zip);
 
         case 'download_mbox':
@@ -556,6 +558,7 @@ public function download(Horde_Variables $vars)
 
         case 'download_render':
             $view_ob = new IMP_Contents_View(new IMP_Indices_Mailbox($vars));
+            $view_ob->checkToken($vars);
             return $view_ob->downloadRender($vars->id, $vars->mode, $vars->ctype);
 
         case 'save_message':

diff --git a/imp/lib/Basic/Folders.php b/imp/lib/Basic/Folders.php
@@ -124,7 +124,7 @@ protected function _init()
 
         case 'download_mbox':
         case 'download_mbox_zip':
-            $registry->downloadUrl('mbox', array(
+            IMP_Contents_View::downloadUrl('mbox', array(
                 'actionID' => 'download_mbox',
                 'mbox_list' => $this->vars->mbox_list,
                 'type' => ($this->vars->actionID == 'download_mbox') ? 'mbox' : 'mboxzip'

diff --git a/imp/lib/Basic/Message.php b/imp/lib/Basic/Message.php
@@ -699,7 +699,7 @@ protected function _init()
 
         $imp_params = $mailbox->urlParams($buid);
         $a_view->save_as = Horde::widget(array(
-            'url' => $registry->downloadUrl($subject, array_merge(array('actionID' => 'save_message'), $imp_params)),
+            'url' => IMP_Contents_View::downloadUrl($subject, array_merge(array('actionID' => 'save_message'), $imp_params)),
             'title' => _("Sa_ve as"),
             'nocheck' => true
         ));

diff --git a/imp/lib/Contents.php b/imp/lib/Contents.php
@@ -881,7 +881,7 @@ public function urlView($mime_part = null, $actionID = 'view_attach',
         $params = $this->_urlViewParams($mime_part, $actionID, isset($options['params']) ? $options['params'] : array());
 
         return (strpos($actionID, 'download_') === 0)
-            ? $GLOBALS['registry']->downloadUrl($mime_part->getName(true), $params)
+            ? IMP_Contents_View::downloadUrl($mime_part->getName(true), $params)
             : Horde::url('view.php', true)->add($params);
     }
 
@@ -906,7 +906,7 @@ protected function _urlViewParams($mime_part, $actionID, $params)
             $params['muid'] = strval($this->getIndicesOb());
         }
 
-        return $params;
+        return IMP_Contents_View::addToken($params);
     }
 
     /**

diff --git a/imp/lib/Contents/View.php b/imp/lib/Contents/View.php
@@ -22,6 +22,9 @@
  */
 class IMP_Contents_View
 {
+    const VIEW_TOKEN = 'imp.view';
+    const VIEW_TOKEN_PARAM = 'view_token';
+
     /**
      * @var IMP_Contents
      */
@@ -315,4 +318,59 @@ public function printAttach($id)
         );
     }
 
+    /**
+     * Check for a download token.
+     *
+     * @param Horde_Variables $vars  Form variables.
+     *
+     * @throws Horde_Token_Exception  Exception on incorrect token.
+     */
+    public function checkToken(Horde_Variables $vars)
+    {
+        global $injector;
+
+        $injector->getInstance('Horde_Token')->validate(
+            $vars->get(self::VIEW_TOKEN_PARAM),
+            self::VIEW_TOKEN
+        );
+    }
+
+    /* Static methods. */
+
+    /**
+     * Returns a URL to be used for downloading data.
+     * IMP adds token data, since the data displayed is coming from a remote
+     * source.
+     *
+     * @see Horde_Registry#downloadUrl()
+     *
+     * @param string $filename  The filename of the download data.
+     * @param array $params     Additional URL parameters needed.
+     *
+     * @return Horde_Url  The download URL.
+     */
+    static public function downloadUrl($filename, array $params = array())
+    {
+        global $registry;
+
+        return $registry->downloadUrl($filename, self::addToken($params));
+    }
+
+    /**
+     * Adds the view token to a parameter list.
+     *
+     * @param array $params  URL parameters.
+     *
+     * @return array  Parameter list with token added.
+     */
+    static public function addToken(array $params = array())
+    {
+        global $injector;
+
+        $params[self::VIEW_TOKEN_PARAM] =
+            $injector->getInstance('Horde_Token')->get(self::VIEW_TOKEN);
+
+        return $params;
+    }
+
 }
diff --git a/imp/lib/Dynamic/Base.php b/imp/lib/Dynamic/Base.php
@@ -133,7 +133,7 @@ protected function _addBaseVars()
             // URL variables
             'MAX_FILE_SIZE' => intval($session->get('imp', 'file_upload')),
             'URI_COMPOSE' => strval(IMP_Dynamic_Compose::url()->setRaw(true)),
-            'URI_VIEW' => strval(Horde::url('view.php')),
+            'URI_VIEW' => strval(Horde::url('view.php')->add(IMP_Contents_View::addToken())),
 
             // Other variables
             'disable_compose' => !IMP_Compose::canCompose()
@@ -197,5 +197,4 @@ static public function url(array $opts = array())
      */
     abstract protected function _init();
 
-
 }
diff --git a/imp/view.php b/imp/view.php
@@ -46,11 +46,13 @@
 
 case 'print_attach':
     $view_ob = new IMP_Contents_View(new IMP_Indices_Mailbox($vars));
+    $view_ob->checkToken($vars);
     $res = $view_ob->printAttach($vars->id);
     break;
 
 case 'view_attach':
     $view_ob = new IMP_Contents_View(new IMP_Indices_Mailbox($vars));
+    $view_ob->checkToken($vars);
     $res = $view_ob->viewAttach($vars->id, $vars->mode, $vars->autodetect, $vars->ctype);
     break;