From 1f38ec70afbc9f47667ae4616b5b642d9a09f3f5 Mon Sep 17 00:00:00 2001
From: Peter Rotich <peter@osticket.com>
Date: Thu, 2 Oct 2014 20:12:58 +0000
Subject: [PATCH] Fix attachment fetching to be object based
---
attachment.php | 14 +++++++++++---
include/class.attachment.php | 22 ++++++++++++++++------
scp/attachment.php | 10 ++++++++--
3 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/attachment.php b/attachment.php
index 8e45d8e15..dc26a66f4 100644
--- a/attachment.php
+++ b/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();
diff --git a/include/class.attachment.php b/include/class.attachment.php
index e7e4c073f..90c168beb 100644
--- a/include/class.attachment.php
+++ b/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);
diff --git a/scp/attachment.php b/scp/attachment.php
index ab6d45ab1..02194c541 100644
--- a/scp/attachment.php
+++ b/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..
--
GitLab