Skip to content

Commit

Permalink
Merge pull request #56 from SilbinaryWolf/fix-filesecurity
Browse files Browse the repository at this point in the history
fix(Security): Store uploaded files in a 'validFileIDs' session and ensure that when the user submits the form, that they uploaded the files and aren't trying to induce quirky behaviour.
  • Loading branch information
Aaron Carlino authored Sep 20, 2016
2 parents bb9906f + c88bdc4 commit 58d36f5
Showing 1 changed file with 101 additions and 0 deletions.
101 changes: 101 additions & 0 deletions code/FileAttachmentField.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ class FileAttachmentField extends FileField {
*/
protected $displayFolderName;

/**
* Set to true if detected invalid file ID
* @var boolean
*/
protected $hasInvalidFileID;

/**
* Helper function to translate underscore_case to camelCase
* @param string $str
Expand Down Expand Up @@ -310,6 +316,91 @@ public function setMaxThumbnailFilesize($num) {
return $this;
}

/**
* Get an associative array of File IDs uploaded through this field
* during this session.
*
* @return array
*/
public function getValidFileIDs() {
$validIDs = Session::get('FileAttachmentField.validFileIDs');
if ($validIDs && is_array($validIDs)) {
return $validIDs;
}
return array();
}

/**
* Check that the user is submitting the file IDs that they uploaded.
*
* @return boolean
*/
public function validate($validator) {
if ($this->hasInvalidFileID) {
// If detected invalid file during 'Form::loadDataFrom'
// (Below validation isn't triggered as setValue() removes the invalid ID
// to prevent the CMS from loading something it shouldn't, also stops the
// validator from realizing there's an invalid ID.)
$validator->validationError(
$this->name,
_t(
'FileAttachmentField.VALIDATION',
'Invalid file ID sent.',
array('id' => $id)
),
"validation"
);
return false;
}

if ($value && is_array($value)) {
// Prevent a malicious user from inspecting element and changing
// one of the <input type="hidden"> fields to use an invalid File ID.
$validIDs = $this->getValidFileIDs();
foreach ($value as $id) {
if (!isset($validIDs[$id])) {
if ($validator) {
$validator->validationError(
$this->name,
_t(
'FileAttachmentField.VALIDATION',
'Invalid file ID sent.',
array('id' => $id)
),
"validation"
);
}
return false;
}
}
}
return true;
}

/**
* @return $this
*/
public function setValue($val, $data = array()) {
if ($data && isset($data[$this->getName()])) {
// Prevent Form::loadDataFrom() from loading invalid File IDs
$isInvalid = false;
$validIDs = $this->getValidFileIDs();
$ids = &$data[$this->getName()];
foreach ($ids as $i => $id) {
if ($validIDs && !isset($validIDs[$id])) {
unset($ids[$i]);
$isInvalid = true;
}
}
if ($isInvalid) {
$ids = array_values($ids);
$val = $ids;
$this->hasInvalidFileID = true;
}
}
return parent::setValue($val, $data);
}

/**
* The thumbnail width
* @param int $num
Expand Down Expand Up @@ -581,6 +672,16 @@ public function upload(SS_HTTPRequest $request) {
}
}

if ($ids) {
$validIDs = Session::get('FileAttachmentField.validFileIDs');
if (!$validIDs) {
$validIDs = array();
}
foreach ($ids as $id) {
$validIDs[$id] = $id;
}
Session::set('FileAttachmentField.validFileIDs', $validIDs);
}
return new SS_HTTPResponse(implode(',', $ids), 200);
}

Expand Down

0 comments on commit 58d36f5

Please sign in to comment.