Fix attachment fetching to be object based

attachment.php

@@ -16,7 +16,7 @@
 **********************************************************************/
 require('secure.inc.php');
 require_once(INCLUDE_DIR.'class.attachment.php');
-// Basic checks
+//Basic checks
 if (!$thisclient
         || !$_GET['id']
         || !$_GET['h']
@@ -30,8 +30,16 @@ if (!$thisclient
         )
     Http::response(404, __('Unknown or invalid file'));
 
-if (!$ticket->checkUserAccess($thisclient))
-    die(__('Access Denied'));
+//Validate session access hash - we want to make sure the link is FRESH! and the user has access to the parent ticket!!
+$vhash=md5($attachment->getFileId().session_id().strtolower($file->getKey()));
+if (strcasecmp(trim($_GET['h']), $vhash)
+        || !($thread=$attachment->getThread())
+        || !($object=$thread->getObject())
+        || !$object instanceof Ticket
+        || !$object->checkUserAccess($thisclient))
+    Http::response(404, __('Unknown or invalid file'));
+//Download the file..
+$file->download();
 
 // Download the file..
 $file->download();

include/class.attachment.php

@@ -21,13 +21,15 @@ class Attachment {
     var $file_id;
 
     var $ht;
-    var $object;
+    var $thread;
 
     function Attachment($id, $tid=0) {
 
-        $sql = ' SELECT * FROM '.THREAD_ENTRY_ATTACHMENT_TABLE.' WHERE id='.db_input($id);
+        $sql = 'SELECT a.*, e.thread_id FROM '.THREAD_ENTRY_ATTACHMENT_TABLE.' a '
+             . 'LEFT JOIN '.THREAD_ENTRY_TABLE.' e ON (e.id = a.thread_entry_id) '
+             . 'WHERE a.id='.db_input($id);
         if($tid)
-            $sql.=' AND thread_entry_id='.db_input($tid);
+            $sql.=' AND a.thread_entry_id='.db_input($tid);
 
         if(!($res=db_query($sql)) || !db_num_rows($res))
             return false;
@@ -37,7 +39,7 @@ class Attachment {
         $this->id=$this->ht['id'];
         $this->file_id=$this->ht['file_id'];
 
-        $this->file=null;
+        $this->file = $this->thread = null;
 
         return true;
     }
@@ -65,8 +67,16 @@ class Attachment {
         return $this->getHashtable();
     }
 
+    function getThread() {
+
+        if (!isset($this->thread))
+            $this->thread = Thread::lookup($this->ht['thread_id']);
+
+        return $this->thread;
+    }
+
     /* Static functions */
-    function getIdByFileHash($hash, $tid=0) {
+    static function getIdByFileHash($hash, $tid=0) {
         $sql='SELECT a.id FROM '.THREAD_ENTRY_ATTACHMENT_TABLE.' a '
             .' INNER JOIN '.FILE_TABLE.' f ON(f.id=a.file_id) '
             .' WHERE f.`key`='.db_input($hash);
@@ -76,7 +86,7 @@ class Attachment {
         return db_result(db_query($sql));
     }
 
-    function lookup($var, $tid=0) {
+    static function lookup($var, $tid=0) {
 
         $id = is_numeric($var) ? $var : self::getIdByFileHash($var, $tid);
 

scp/attachment.php

@@ -16,7 +16,7 @@
 require('staff.inc.php');
 require_once(INCLUDE_DIR.'class.attachment.php');
 
-// Basic checks
+//Basic checks
 if (!$thisstaff
         || !$_GET['id']
         || !$_GET['h']
@@ -30,7 +30,13 @@ if (!$thisstaff
         )
     Http::response(404, __('Unknown or invalid file'));
 
-if (!$ticket->checkStaffAccess($thisstaff))
+//Validate session access hash - we want to make sure the link is FRESH! and the user has access to the parent ticket!!
+$vhash=md5($attachment->getFileId().session_id().strtolower($file->getKey()));
+if (strcasecmp(trim($_GET['h']), $vhash)
+        || !($thread=$attachment->getThread())
+        || !($object=$thread->getObject())
+        || !$object instanceof Ticket
+        || !$object->checkStaffAccess($thisstaff))
     die(__('Access Denied'));
 
 //Download the file..