Skip to content

Commit

Permalink
fix(Security): Store uploaded files in a 'validFileIDs' session and e…
Browse files Browse the repository at this point in the history
…nsure that when the user submits the form, that they uploaded the files and aren't trying to induce quirky behaviour.
  • Loading branch information
Jake Bentvelzen committed Sep 16, 2016
1 parent bb9906f commit c88bdc4
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 c88bdc4

Please sign in to comment.