From 2582d2272be177fc084a83459ab55adf8a4ddf4e Mon Sep 17 00:00:00 2001 From: glubsy Date: Sat, 23 Jan 2021 18:30:19 +0000 Subject: [PATCH 1/5] Limit thumbnail generation * This fixes a denial-of-service exploit that would allow the client to generate an infinite number of thumbnails and fill up the storage completely. Since the client had control over the requested thumbnail sizes, it could make arbitrary requests for thumbnails, and every time the backend did not find an already generated thumbnail with the specified sizes, it would happily generate a new one. * Remove the ability of the client to decide thumbnail dimensions and only let the back-end do this by reading the configuration. * Limit the number of generated thumbnails per file to only one, with "landscape" dimensions (4/3). * Use CSS "object-fit" property to adjust displaying of landscape thumbnails into squares. Ref: https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit --- src/_h5ai/private/conf/options.json | 2 +- src/_h5ai/private/php/core/class-context.php | 5 ++++- src/_h5ai/private/php/ext/class-thumb.php | 4 ++-- src/_h5ai/public/css/lib/view/view.less | 2 ++ src/_h5ai/public/js/lib/ext/thumbnails.js | 5 ----- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/_h5ai/private/conf/options.json b/src/_h5ai/private/conf/options.json index a570da26f..2e9e83d14 100644 --- a/src/_h5ai/private/conf/options.json +++ b/src/_h5ai/private/conf/options.json @@ -355,7 +355,7 @@ - mov: array of strings - doc: array of strings - delay: number, delay in milliseconds after "dom-ready" before thumb-requesting starts - - size: number, size in pixel of the generated thumbnails + - size: number, height in pixel of the generated thumbnails - exif: boolean, use included EXIF thumbs if possible - chunksize: int, number of thumbs per request */ diff --git a/src/_h5ai/private/php/core/class-context.php b/src/_h5ai/private/php/core/class-context.php index 0a72cea54..64c9b72cc 100644 --- a/src/_h5ai/private/php/core/class-context.php +++ b/src/_h5ai/private/php/core/class-context.php @@ -23,6 +23,9 @@ public function __construct($session, $request, $setup) { $this->options = Json::load($this->setup->get('CONF_PATH') . '/options.json'); + $this->thumbnail_height = $this->options['thumbnails']['size'] ?? 240; + $this->thumbnail_width = floor($this->thumbnail_height * (4 / 3)); + $this->passhash = $this->query_option('passhash', ''); $this->options['hasCustomPasshash'] = strcasecmp($this->passhash, Context::$DEFAULT_PASSHASH) !== 0; unset($this->options['passhash']); @@ -250,7 +253,7 @@ public function get_thumbs($requests) { foreach ($requests as $req) { $thumb = new Thumb($this); - $hrefs[] = $thumb->thumb($req['type'], $req['href'], $req['width'], $req['height']); + $hrefs[] = $thumb->thumb($req['type'], $req['href']); } return $hrefs; diff --git a/src/_h5ai/private/php/ext/class-thumb.php b/src/_h5ai/private/php/ext/class-thumb.php index d52d3e85e..0f9823fb4 100644 --- a/src/_h5ai/private/php/ext/class-thumb.php +++ b/src/_h5ai/private/php/ext/class-thumb.php @@ -23,7 +23,7 @@ public function __construct($context) { } } - public function thumb($type, $source_href, $width, $height) { + public function thumb($type, $source_href) { $source_path = $this->context->to_path($source_href); if (!file_exists($source_path) || Util::starts_with($source_path, $this->setup->get('CACHE_PUB_PATH'))) { return null; @@ -46,7 +46,7 @@ public function thumb($type, $source_href, $width, $height) { } } - return $this->thumb_href($capture_path, $width, $height); + return $this->thumb_href($capture_path, $this->context->thumbnail_width, $this->context->thumbnail_height); } private function thumb_href($source_path, $width, $height) { diff --git a/src/_h5ai/public/css/lib/view/view.less b/src/_h5ai/public/css/lib/view/view.less index aef93ecbb..1be5170dc 100644 --- a/src/_h5ai/public/css/lib/view/view.less +++ b/src/_h5ai/public/css/lib/view/view.less @@ -48,6 +48,8 @@ .thumb { max-width: none; max-height: none; + object-fit: cover; + object-position: 50% 50%; } } diff --git a/src/_h5ai/public/js/lib/ext/thumbnails.js b/src/_h5ai/public/js/lib/ext/thumbnails.js index d71898cc2..d92c14554 100644 --- a/src/_h5ai/public/js/lib/ext/thumbnails.js +++ b/src/_h5ai/public/js/lib/ext/thumbnails.js @@ -13,7 +13,6 @@ const settings = Object.assign({ exif: false, chunksize: 20 }, allsettings.thumbnails); -const landscapeRatio = 4 / 3; const queueItem = (queue, item) => { @@ -35,7 +34,6 @@ const queueItem = (queue, item) => { queue.push({ type, href: item.absHref, - ratio: 1, callback: src => { if (src && item.$view) { item.thumbSquare = src; @@ -51,7 +49,6 @@ const queueItem = (queue, item) => { queue.push({ type, href: item.absHref, - ratio: landscapeRatio, callback: src => { if (src && item.$view) { item.thumbRational = src; @@ -67,8 +64,6 @@ const requestQueue = queue => { return { type: req.type, href: req.href, - width: Math.round(settings.size * req.ratio), - height: settings.size }; }); From a2d21cfdeeac726e5b78336c9d7b0c1b23eb60ac Mon Sep 17 00:00:00 2001 From: glubsy Date: Mon, 25 Jan 2021 01:51:54 +0000 Subject: [PATCH 2/5] Remove redundant request for square thumb * We now use only one thumbnail file src for both square and rational thumbnails so remove superfluous requests. * Set the same src for both square and landscape dom classes. --- .../public/js/lib/ext/preview/preview.js | 2 +- src/_h5ai/public/js/lib/ext/thumbnails.js | 19 ++----------------- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/_h5ai/public/js/lib/ext/preview/preview.js b/src/_h5ai/public/js/lib/ext/preview/preview.js index 703b2b995..d5a44fb07 100644 --- a/src/_h5ai/public/js/lib/ext/preview/preview.js +++ b/src/_h5ai/public/js/lib/ext/preview/preview.js @@ -213,7 +213,7 @@ Session.prototype = { } }); dom('#pv-container').hide().clr(); - showSpinner(true, item.thumbSquare || item.icon, 200); + showSpinner(true, item.thumbRational || item.icon, 200); }) .then(() => this.load(item)) // delay for testing diff --git a/src/_h5ai/public/js/lib/ext/thumbnails.js b/src/_h5ai/public/js/lib/ext/thumbnails.js index d92c14554..a8ef6a614 100644 --- a/src/_h5ai/public/js/lib/ext/thumbnails.js +++ b/src/_h5ai/public/js/lib/ext/thumbnails.js @@ -28,23 +28,8 @@ const queueItem = (queue, item) => { return; } - if (item.thumbSquare) { - item.$view.find('.icon.square img').addCls('thumb').attr('src', item.thumbSquare); - } else { - queue.push({ - type, - href: item.absHref, - callback: src => { - if (src && item.$view) { - item.thumbSquare = src; - item.$view.find('.icon.square img').addCls('thumb').attr('src', src); - } - } - }); - } - if (item.thumbRational) { - item.$view.find('.icon.landscape img').addCls('thumb').attr('src', item.thumbRational); + item.$view.find('.icon img').addCls('thumb').attr('src', item.thumbRational); } else { queue.push({ type, @@ -52,7 +37,7 @@ const queueItem = (queue, item) => { callback: src => { if (src && item.$view) { item.thumbRational = src; - item.$view.find('.icon.landscape img').addCls('thumb').attr('src', src); + item.$view.find('.icon img').addCls('thumb').attr('src', src); } } }); From e8761848613fdc15d10469580019c33587d18033 Mon Sep 17 00:00:00 2001 From: glubsy Date: Mon, 25 Jan 2021 21:42:41 +0000 Subject: [PATCH 3/5] Reduce superfluous computations Only compute thumbnail configured dimensions on thumbnail API requests. --- src/_h5ai/private/php/core/class-context.php | 7 +++---- src/_h5ai/private/php/ext/class-thumb.php | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/_h5ai/private/php/core/class-context.php b/src/_h5ai/private/php/core/class-context.php index 64c9b72cc..071b56af3 100644 --- a/src/_h5ai/private/php/core/class-context.php +++ b/src/_h5ai/private/php/core/class-context.php @@ -23,9 +23,6 @@ public function __construct($session, $request, $setup) { $this->options = Json::load($this->setup->get('CONF_PATH') . '/options.json'); - $this->thumbnail_height = $this->options['thumbnails']['size'] ?? 240; - $this->thumbnail_width = floor($this->thumbnail_height * (4 / 3)); - $this->passhash = $this->query_option('passhash', ''); $this->options['hasCustomPasshash'] = strcasecmp($this->passhash, Context::$DEFAULT_PASSHASH) !== 0; unset($this->options['passhash']); @@ -250,10 +247,12 @@ public function get_l10n($iso_codes) { public function get_thumbs($requests) { $hrefs = []; + $height = $this->options['thumbnails']['size'] ?? 240; + $width = floor($this->thumbnail_height * (4 / 3)); foreach ($requests as $req) { $thumb = new Thumb($this); - $hrefs[] = $thumb->thumb($req['type'], $req['href']); + $hrefs[] = $thumb->thumb($req['type'], $req['href'], $width, $height); } return $hrefs; diff --git a/src/_h5ai/private/php/ext/class-thumb.php b/src/_h5ai/private/php/ext/class-thumb.php index 0f9823fb4..d52d3e85e 100644 --- a/src/_h5ai/private/php/ext/class-thumb.php +++ b/src/_h5ai/private/php/ext/class-thumb.php @@ -23,7 +23,7 @@ public function __construct($context) { } } - public function thumb($type, $source_href) { + public function thumb($type, $source_href, $width, $height) { $source_path = $this->context->to_path($source_href); if (!file_exists($source_path) || Util::starts_with($source_path, $this->setup->get('CACHE_PUB_PATH'))) { return null; @@ -46,7 +46,7 @@ public function thumb($type, $source_href) { } } - return $this->thumb_href($capture_path, $this->context->thumbnail_width, $this->context->thumbnail_height); + return $this->thumb_href($capture_path, $width, $height); } private function thumb_href($source_path, $width, $height) { From ba6b01ab7caa40de06ebcd44af4f7c468e246c97 Mon Sep 17 00:00:00 2001 From: glubsy Date: Tue, 26 Jan 2021 02:30:05 +0000 Subject: [PATCH 4/5] Fix typo. --- src/_h5ai/private/php/core/class-context.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_h5ai/private/php/core/class-context.php b/src/_h5ai/private/php/core/class-context.php index 071b56af3..7ced778c5 100644 --- a/src/_h5ai/private/php/core/class-context.php +++ b/src/_h5ai/private/php/core/class-context.php @@ -248,7 +248,7 @@ public function get_l10n($iso_codes) { public function get_thumbs($requests) { $hrefs = []; $height = $this->options['thumbnails']['size'] ?? 240; - $width = floor($this->thumbnail_height * (4 / 3)); + $width = floor($height * (4 / 3)); foreach ($requests as $req) { $thumb = new Thumb($this); From 4a3dd108bd20cb598958c5563c2098629ffd9e03 Mon Sep 17 00:00:00 2001 From: glubsy Date: Sat, 6 Feb 2021 04:10:00 +0000 Subject: [PATCH 5/5] Remove client-side sample request for img * Down-sampling should only be requested by the backend. * For now remove the client-side logic entirely since it is both superfluous and vulnerable. This can be implemented again on the back-end if necessary. --- src/_h5ai/private/conf/options.json | 2 -- .../public/js/lib/ext/preview/preview-img.js | 30 ++++--------------- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/_h5ai/private/conf/options.json b/src/_h5ai/private/conf/options.json index 2e9e83d14..af6cdc53b 100644 --- a/src/_h5ai/private/conf/options.json +++ b/src/_h5ai/private/conf/options.json @@ -234,11 +234,9 @@ Show an image preview on click. - types: array of strings - - size: number, sample size, or false for original size */ "preview-img": { "enabled": true, - "size": false, "types": ["img", "img-bmp", "img-gif", "img-ico", "img-jpg", "img-png", "img-raw", "img-svg"] }, diff --git a/src/_h5ai/public/js/lib/ext/preview/preview-img.js b/src/_h5ai/public/js/lib/ext/preview/preview-img.js index 1b4b5a5c4..bf8277f9d 100644 --- a/src/_h5ai/public/js/lib/ext/preview/preview-img.js +++ b/src/_h5ai/public/js/lib/ext/preview/preview-img.js @@ -1,11 +1,9 @@ const {dom} = require('../../util'); -const server = require('../../server'); const allsettings = require('../../core/settings'); const preview = require('./preview'); const settings = Object.assign({ enabled: false, - size: null, types: [] }, allsettings['preview-img']); const tpl = ''; @@ -19,34 +17,16 @@ const updateGui = () => { const elW = el.offsetWidth; const labels = [preview.item.label]; - if (!settings.size) { - const elNW = el.naturalWidth; - const elNH = el.naturalHeight; - labels.push(String(elNW) + 'x' + String(elNH)); - labels.push(String((100 * elW / elNW).toFixed(0)) + '%'); - } - preview.setLabels(labels); -}; + const elNW = el.naturalWidth; + const elNH = el.naturalHeight; + labels.push(String(elNW) + 'x' + String(elNH)); + labels.push(String((100 * elW / elNW).toFixed(0)) + '%'); -const requestSample = href => { - return server.request({ - action: 'get', - thumbs: [{ - type: 'img', - href, - width: settings.size, - height: 0 - }] - }).then(json => { - return json && json.thumbs && json.thumbs[0] ? json.thumbs[0] : null; - }); + preview.setLabels(labels); }; const load = item => { return Promise.resolve(item.absHref) - .then(href => { - return settings.size ? requestSample(href) : href; - }) .then(href => new Promise(resolve => { const $el = dom(tpl) .on('load', () => resolve($el))