Skip to content

Commit

Permalink
Merge pull request from GHSA-9qgp-9wwc-v29r
Browse files Browse the repository at this point in the history 
User needs to have rights to see /uploads/* content
  • Loading branch information
@atomiix
atomiix committed Dec 6, 2022
2 parents 8d82ed9 + fa56210 commit 8684d42
Show file tree
Hide file tree
Showing 9 changed files with 332 additions and 185 deletions.
2 changes: 1 addition & 1 deletion admin-dev/themes/default/template/controllers/customer_threads/helpers/view/message.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
</h4>
{/if}
<span class="message-date">&nbsp;<i class="icon-calendar"></i> - {dateFormat date=$message.date_add full=0} - <i class="icon-time"></i> {$message.date_add|substr:11:5}</span>
{if isset($message.file_name)} <span class="message-product">&nbsp;<i class="icon-link"></i> <a href="{$message.file_name|escape:'html':'UTF-8'}" target="_blank" rel="noopener noreferrer nofollow">{l s="Attachment" d='Admin.Catalog.Feature'}</a></span>{/if}
{if isset($message.file_name)} <span class="message-product">&nbsp;<i class="icon-link"></i> <a href="{$message.file_link|escape:'html':'UTF-8'}" target="_blank" rel="noopener noreferrer nofollow">{l s="Attachment" d='Admin.Catalog.Feature'}</a></span>{/if}
{if isset($message.product_name)} <span class="message-attachment">&nbsp;<i class="icon-book"></i> <a href="{$message.product_link|escape:'html':'UTF-8'}" target="_blank" rel="noopener noreferrer nofollow">{l s="Product" d='Admin.Global'} {$message.product_name|escape:'html':'UTF-8'} </a></span>{/if}
<p class="message-item-text">{$message.message|escape:'html':'UTF-8'|nl2br}</p>
</div>
Expand Down
18 changes: 13 additions & 5 deletions classes/Dispatcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ class DispatcherCore
* @var array List of default routes
*/
public $default_routes = [
'upload' => [
'controller' => 'upload',
'rule' => 'upload/{file}',
'keywords' => [
'file' => ['regexp' => '.+', 'param' => 'file'],
],
],
'category_rule' => [
'controller' => 'category',
'rule' => '{id}-{rewrite}',
Expand Down Expand Up @@ -1029,12 +1036,13 @@ public function getController($id_shop = null)
$controller = $this->controller_not_found;
$test_request_uri = preg_replace('/(=http:\/\/)/', '=', $this->request_uri);

// If the request_uri matches a static file, then there is no need to check the routes, we keep
// If the request_uri matches a static file, unless it's in the upload folder,
// then there is no need to check the routes, we keep
// "controller_not_found" (a static file should not go through the dispatcher)
if (!preg_match(
'/\.(gif|jpe?g|png|css|js|ico)$/i',
parse_url($test_request_uri, PHP_URL_PATH)
)) {
if (
!preg_match('/\.(gif|jpe?g|png|css|js|ico)$/i', parse_url($test_request_uri, PHP_URL_PATH))
|| preg_match('/^\/upload/', parse_url($test_request_uri, PHP_URL_PATH)))
{
// Add empty route as last route to prevent this greedy regexp to match request uri before right time
if ($this->empty_route) {
$this->addRoute(
Expand Down
4 changes: 3 additions & 1 deletion classes/Tools.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2632,7 +2632,9 @@ public static function generateHtaccess($path = null, $rewrite_settings = null,
fwrite($write_fd, 'RewriteRule . - [E=REWRITEBASE:' . $uri['physical'] . ']' . PHP_EOL);

// Webservice
fwrite($write_fd, 'RewriteRule ^api(?:/(.*))?$ %{ENV:REWRITEBASE}webservice/dispatcher.php?url=$1 [QSA,L]' . "\n\n");
fwrite($write_fd, 'RewriteRule ^api(?:/(.*))?$ %{ENV:REWRITEBASE}webservice/dispatcher.php?url=$1 [QSA,L]' . PHP_EOL);
// upload folder
fwrite($write_fd, 'RewriteRule ^upload/.+$ %{ENV:REWRITEBASE}index.php [QSA,L]' . "\n\n");

if (!$rewrite_settings) {
$rewrite_settings = (int) Configuration::get('PS_REWRITING_SETTINGS', null, null, (int) $uri['id_shop']);
Expand Down
30 changes: 23 additions & 7 deletions controllers/admin/AdminCustomerThreadsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,14 +512,18 @@ public function postProcess()

public function initContent()
{
if (isset($_GET['filename']) && file_exists(_PS_UPLOAD_DIR_ . $_GET['filename']) && Validate::isFileName($_GET['filename'])) {
AdminCustomerThreadsController::openUploadedFile();
if (isset($_GET['filename'])) {
if (file_exists(_PS_UPLOAD_DIR_ . $_GET['filename']) && Validate::isFileName($_GET['filename'])) {
$this->openUploadedFile(!Tools::getValue('show'));
} else {
Tools::redirect('404');
}
}

return parent::initContent();
}

protected function openUploadedFile()
protected function openUploadedFile(bool $forceDownload = true)
{
$filename = $_GET['filename'];

Expand Down Expand Up @@ -553,7 +557,9 @@ protected function openUploadedFile()
ob_end_clean();
}
header('Content-Type: ' . $extension);
header('Content-Disposition:attachment;filename="' . $filename . '"');
if ($forceDownload) {
header('Content-Disposition:attachment;filename="' . $filename . '"');
}
readfile(_PS_UPLOAD_DIR_ . $filename);
die;
}
Expand Down Expand Up @@ -631,10 +637,20 @@ public function renderView()
$employee = new Employee($mess['id_employee']);
$messages[$key]['employee_image'] = $employee->getImage();
}
if (isset($mess['file_name']) && $mess['file_name'] != '') {
$messages[$key]['file_name'] = _THEME_PROD_PIC_DIR_ . $mess['file_name'];
} else {
if (empty($mess['file_name'])) {
unset($messages[$key]['file_name']);
} else {
$messages[$key]['file_link'] = $this->context->link->getAdminLink(
'AdminCustomerThreads',
true,
[],
[
'id_customer_thread' => $id_customer_thread,
'viewcustomer_thread' => '',
'filename' => $mess['file_name'],
'show' => true,
]
);
}

if ($mess['id_product']) {
Expand Down
Loading

0 comments on commit 8684d42

Please sign in to comment.