From 2053740834354a792a1be4254f558e564d4d023a Mon Sep 17 00:00:00 2001 From: Jared Hancock Date: Sat, 3 Oct 2015 15:14:11 -0500 Subject: [PATCH] files: Only allow files uploaded in this session This fixes a security issue where, by crafting a special POST request to the client open.php page, an (unauthenticated) user could get a URL link to access to any attachment already uploaded in the system by guessing or brute-forcing the file's ID number. This patch addresses the issue by registering the uploaded file's ID in the current user's session. When processing the list of file ID's attached to the FileUploadField, the files must already have been attached to the field or have been newly attached in the current session. Fixes #2615 References: "Security issue - Download attachments submitted by others" https://github.com/osTicket/osTicket-1.8/issues/2615 --- include/class.forms.php | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/include/class.forms.php b/include/class.forms.php index 43d1416ffa..91cf301fa1 100644 --- a/include/class.forms.php +++ b/include/class.forms.php @@ -1517,6 +1517,9 @@ function ajaxUpload($bypass=false) { if (!($id = AttachmentFile::upload($file))) Http::response(500, 'Unable to store file: '. $file['error']); + // This file is allowed for attachment in this session + $_SESSION[':uploadedFiles'][$id] = 1; + return $id; } @@ -2206,7 +2209,27 @@ function getValue() { elseif ($data && is_array($data) && !isset($data[$this->name])) return array(); - return parent::getValue(); + + // Files uploaded here MUST have been uploaded by this user and + // identified in the session + if ($files = parent::getValue()) { + $allowed = array(); + // Files already attached to the field are allowed + foreach ($this->field->getFiles() as $F) { + // FIXME: This will need special porting in v1.10 + $allowed[$F['id']] = 1; + } + // New files uploaded in this session are allowed + if (isset($_SESSION[':uploadedFiles'])) { + $allowed += $_SESSION[':uploadedFiles']; + } + foreach ($files as $i=>$F) { + if (!isset($allowed[$F])) { + unset($files[$i]); + } + } + } + return $files; } }