Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core, Media] Interface for safe file downloading #3881

Closed
wants to merge 35 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f254050
Generic downloader. Needs work
johnsaigle Aug 14, 2018
9328912
Remove typos
johnsaigle Aug 15, 2018
0d62aad
Generic file downloader proof-of-concept
johnsaigle Aug 15, 2018
e0d21b3
cleanup
johnsaigle Aug 15, 2018
fa14a23
phpcbf
johnsaigle Aug 15, 2018
eb267b9
PHPDoc and phpcs for GenericDownloader
johnsaigle Aug 15, 2018
516b90c
Fix documentation typos
johnsaigle Aug 21, 2018
294216a
Throw exception instead of exit
johnsaigle Aug 21, 2018
eb9fd44
Remove function from interface
johnsaigle Aug 21, 2018
f4a7830
400 to 404 and don't exit
johnsaigle Aug 21, 2018
1ad703e
Documentation and phpcs
johnsaigle Aug 21, 2018
0bf572a
Exception typo
johnsaigle Aug 27, 2018
8de4bb0
Refactor MIME type functionality
johnsaigle Aug 27, 2018
80bc363
StreamInterface usage
johnsaigle Aug 27, 2018
50fcfbb
Fix phan errors
johnsaigle Aug 29, 2018
fe1f02d
lint error
johnsaigle Aug 29, 2018
f95973e
Symlinks should work now
johnsaigle Sep 11, 2018
8c95133
Sanitize path when checking mime type
johnsaigle Sep 17, 2018
f46eff2
Typo
johnsaigle Sep 17, 2018
ab243fc
Refactor code to use more Diactoros functionality
johnsaigle Sep 17, 2018
310bef4
Remove unused PSR references
johnsaigle Sep 17, 2018
47a15b0
Typo var name
johnsaigle Sep 17, 2018
3957473
phpcs
johnsaigle Sep 17, 2018
c249e22
Cleanup
johnsaigle Sep 18, 2018
0bc8da7
phpcs GetFile
johnsaigle Sep 18, 2018
30f1815
Remove dead code
johnsaigle Sep 19, 2018
f98e354
Make everything more PSR7-y
johnsaigle Sep 19, 2018
b9a9c6f
Documentation and phpcs
johnsaigle Sep 19, 2018
fb2ebeb
Move more functionality into AttachmentResponse
johnsaigle Sep 19, 2018
64dd764
Fix typos in docs
johnsaigle Sep 19, 2018
8b87851
PSR/Dave overhaul, incomplete
johnsaigle Nov 6, 2018
821a6d3
Remove old files
johnsaigle Nov 6, 2018
f16c98d
Restore ajax file to how it was
johnsaigle Nov 6, 2018
f083190
Move most functions to media class
johnsaigle Nov 28, 2018
2fbc3e0
Make validator lower case
johnsaigle Nov 28, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions mediafiledownloadvalidator.class.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
/**
* MediaFileDownloadValidator downloads files and does some validation.
*/
namespace LORIS\media;

require_once 'FileDownloadValidator.class.inc';

class MediaFileDownloadValidator extends FileDownloadValidator {

public function __construct(string $filepath) {
$paths = \NDB_Config::singleton()->getSetting('paths');
/* The mediaPath config setting should be set to the full path to the
* directory containing the requested file.
*/
$this->filepath = $paths['mediaPath'] . $filepath;
error_log($this->filepath);
}

public function getContentType(): string
{
return finfo_open(FILEINFO_MIME, $this->filepath);
}

public function exists(): bool
{
return file_exists($this->filepath);
}

public function getFilename(): string {
return $this->filepath;
}

}
38 changes: 38 additions & 0 deletions modules/media/php/media.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
* @link https://www.github.com/aces/CCNA/
*/
namespace LORIS\media;
use \Psr\Http\Message\ServerRequestInterface;
use \Psr\Http\Message\ResponseInterface;

#require_once __DIR__ . '/MediaFileDownloadValidator.class.inc';
/**
* Main class for media module corresponding to /media/ URL
* Child or Clinical section of the LorisMenu.
Expand Down Expand Up @@ -178,6 +181,41 @@ class Media extends \NDB_Menu_Filter
return true;
}

/**
* This acts as an AJAX endpoint that serves an attachment to the user based
* on the uploads to the media module.
*
* @param ServerRequestInterface $request The incoming PSR7 request
*
* @return ResponseInterface The outgoing PSR7 response
*/
public function handle(ServerRequestInterface $request): ResponseInterface
{
error_log('in handle');
// The parent handles things like hasAccess checks.
$resp = parent::handle($request);
// Ensure filepath is passed.
$gets = $request->getQueryParams();
if (isset($gets['File'])) {
error_log('I see the file');
$this->handleDownload($gets['File']);
}
return $resp;
}

function handleDownload(string $file) {
error_log('in handle download');
$file = new php\MediaFileDownloadValidator($gets['File']);
return (new \Zend\Diactoros\Response())
->withStatus(200)
->withBody(new \LORIS\Http\StringStream("hello"))
->withHeader("Content-Type", "text/plain")
->withHeader(
'Content-Disposition', 'attachment; filename=' .
$file->getFilename()
);
}

/**
* Converts the results of this menu filter to a JSON format to be retrieved
* with ?format=json
Expand Down
104 changes: 104 additions & 0 deletions modules/media/php/mediadownload.class.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php
/**
* The media download endpoint serves file attachments to users based on files
* uploaded to the media module.
*
* It takes a filepath specified by $File and returns a corresponding attachment
* assuming the request is valid and permitted.
*
* PHP Version 7
*
* @category Main
* @package Media
* @author Loris Team <loris.mni@bic.mni.mcgill.ca>
* @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
* @link https://www.github.com/aces/Loris/
*/
namespace LORIS\media;

use \Psr\Http\Message\ServerRequestInterface;
use \Psr\Http\Message\ResponseInterface;

#require_once __DIR__ . '/MediaFileDownloadValidator.class.inc';

/**
* Implements the media file download endpoint.
*
* @category Main
* @package Media
* @author Loris Team <loris.mni@bic.mni.mcgill.ca>
* @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
* @link https://www.github.com/aces/Loris/
*/

class MediaDownload extends \NDB_Page
{
public $skipTemplate = true;
/**
* A user must have the media_write permission to download files.
* FIXME this probably makes more sense as a read permission but for now
* the write and download permissions are coupled.
*
* @return bool true iff the user has access to this page.
*/
function _hasAccess()
{
return \User::singleton()->hasPermission('media_write');
}
/**
* This acts as an AJAX endpoint that serves an attachment to the user based
* on the uploads to the media module.
*
* @param ServerRequestInterface $request The incoming PSR7 request
*
* @return ResponseInterface The outgoing PSR7 response
*/
public function handle(ServerRequestInterface $request): ResponseInterface
{
// The parent handles things like hasAccess checks.
$resp = parent::handle($request);
switch ($resp->getStatusCode()) {
case 200:
// If it was a 200 code, it just means display was called and
// access validated. We still need to do other validations
// and add the result body.
break;
default:
return $resp;
}
// Ensure it's a GET request.
if ($request->getMethod() != "GET") {
return (new \LORIS\Http\Response())
->withHeader("Content-Type", "text/plain")
->withStatus(405)
->withHeader("Allow", "GET")
->withBody(
new \LORIS\Http\StringStream(
"Only GET requests are allowed"
)
);
}
// Ensure filepath is passed.
$gets = $request->getQueryParams();
print_r($gets);
if (!isset($gets['File'])) {
return (new \LORIS\Http\Response())
->withHeader("Content-Type", "text/plain")
->withStatus(400)
->withBody(
new \LORIS\Http\StringStream(
"Must provide a file path"
)
);
}
$file = new MediaFileDownloadValidator($gets['File']);
return (new \Zend\Diactoros\Response())
->withStatus(200)
->withBody(new \LORIS\Http\StringStream("hello"))
->withHeader("Content-Type", "text/plain")
->withHeader(
'Content-Disposition', 'attachment; filename=' .
$file->getFilename()
);
}
}
39 changes: 39 additions & 0 deletions php/libraries/DownloadableFile.class.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php
/**
* A DownloadableFile represents a file that a user
* can download.
*/
interface DownloadableFile {
/**
* Returns a StreamInterface to download the file
*/
public function getContents() : \Psr\Http\Message\StreamInterface;

/**
* Returns the Content-Type being returned, if
* known. The empty string should be interpreted
* as "unknown"
*/
public function getContentType(): string;

/**
* Returns the filename for this file that should
* be presented to the end user, if known, otherwise
* the empty string
*/
public function getFilename(): string;

/**
* Returns true if the file being downloaded exists. Classes implementing
* this interface should ensure that this check covers a basic check of
* whether the requested file exists on the file system as well as additional
* database-related checks as necessary.
*/
public function exists(): bool;

/**
* Return true if the user can access this file
*/
public function hasAccess(\User $user): bool;

}
83 changes: 83 additions & 0 deletions php/libraries/FileDownloadValidator.class.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php
/**
* FileDownloadValidator downloads files and does some validation.
* It does reasonable things for the above interface where applicable
* and leaves the rest abstract for a concrete implementation to
* enforce such as a module checking permissions.
*/
require_once 'DownloadableFile.class.inc';
abstract class FileDownloadValidator implements DownloadableFile {
protected $filepath;

public function __construct(string $filepath) {
$this->filepath = $filepath;
}

public function getContents() : \Psr\Http\Message\StreamInterface {
// an example of a reasonable thing to do here
return new \Zend\Diactoros\Stream($this->filepath);
}

abstract public function getContentType() : string;

/**
* FIXME This function should be expanded to specify which users can
* acess a given file. However this functionality is not yet implemented
* in LORIS. Anyone with te media_write permission can download any file;
* there is no concept of user-file permissions at the LORIS level.
*
* @return bool True
*/
public function hasAccess(\User $user): bool
{
return true;
}

private function _isSafe(): bool
{
// Check for path traversal attempts for file that are not symlinks.
if (!is_link($this->filepath)) {
if (!$this->_safePathPrefix($this->path)) {
return false;
}
}
}

/**
* Protects against path traversal by resolving paths containing relative
* characters. This is done by checking that the prefix of the real path
* matches the one supplied to the function.
*
* E.g.
* Input of "/var/www/loris/my_uploads/" will have the same prefix as
* the valid path of /var/www/loris/my_uploads/, but an input like
* "/var/www/loris/my_uploads/../../../../../etc/passwd" will resolve
* to "/etc/passwd" which will not have the same prefix as the valid
* destination.
* Note that this approach does not work for symlinks as their resolved
* paths are expected to be outside of the target directory. This option
* is left open in case sysadmins decide that the files must be hosted
* elsewhere.
*
* @return bool Whether path traversal is happening.
*/
private function _safePathPrefix(): bool
{
// Resolve path (remove '..' and other relative file path structures.)
$realUserPath = realpath($userPath);
if ($realUserPath === false
|| strpos($realUserPath, $userPath) !== 0
) {
return false;
}
return true;
}

/**
*
* @throws \LorisException
*/
private function _validate(): void
{
}
}