From 052e1c84e6f3886785d70726f2b7cd528976d231 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Tue, 5 Jan 2021 11:21:09 +0100 Subject: [PATCH 1/3] do not repeat successful security checks. fixes #3363 This avoids creating lots of 403 errors for properly secured data directories. Only one successful check per browser session will be executed. --- inc/Ui/Admin.php | 11 ++++++----- lib/scripts/behaviour.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/inc/Ui/Admin.php b/inc/Ui/Admin.php index 07146e598..27a57225c 100644 --- a/inc/Ui/Admin.php +++ b/inc/Ui/Admin.php @@ -28,9 +28,10 @@ class Admin extends Ui { $this->menu = $this->getPluginList(); echo '
'; echo p_locale_xhtml('admin'); - $this->showSecurityCheck(); + $this->showMenu('admin'); $this->showMenu('manager'); + $this->showSecurityCheck(); $this->showVersion(); $this->showMenu('other'); echo '
'; @@ -75,16 +76,16 @@ class Admin extends Ui { * it verifies either: * 'savedir' has been moved elsewhere, or * has protection to prevent the webserver serving files from it + * + * The actual check is carried out via JavaScript. See behaviour.js */ protected function showSecurityCheck() { global $conf; if(substr($conf['savedir'], 0, 2) !== './') return; $img = DOKU_URL . $conf['savedir'] . '/dont-panic-if-you-see-this-in-your-logs-it-means-your-directory-permissions-are-correct.png'; - echo ' - Your data directory seems to be protected properly.'; + echo '⚠'; } /** diff --git a/lib/scripts/behaviour.js b/lib/scripts/behaviour.js index 70b60ef9a..009081f80 100644 --- a/lib/scripts/behaviour.js +++ b/lib/scripts/behaviour.js @@ -56,6 +56,7 @@ var dw_behaviour = { dw_behaviour.checkWindowsShares(); dw_behaviour.subscription(); dw_behaviour.pageRestoreConfirm(); + dw_behaviour.securityCheck(); dw_behaviour.revisionBoxHandler(); jQuery(document).on('click','#page__revisions input[type=checkbox]', @@ -204,6 +205,35 @@ var dw_behaviour = { } }); } + }, + + /** + * Check that access to the data directory is properly secured + * + * A successful check (a 403 error was returned when loading the image) is saved + * to session storage and not repeated again until the next browser session. This + * avoids overeager security bans (see #3363) + */ + securityCheck: function () { + var $checkA = jQuery('#security__check'); + if (!$checkA.length) return; + if (sessionStorage.getItem('security-check')) { + // check was already executed successfully + $checkA.remove(); + return; + } + + var img = new Image(347, 63); + img.onerror = function () { + // successful check will not be repeated during session + $checkA.remove(); + sessionStorage.setItem('security-check', true); + } + img.onload = function () { + // check failed, display the image + $checkA.html(img); + } + img.src = $checkA.data('src'); } }; From d3f829c2e865411af7ac548cd180da21c39771f2 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 11 Jan 2021 17:16:30 +0100 Subject: [PATCH 2/3] cleaner behaviour for data directory check Show a properly translated message. --- inc/Ui/Admin.php | 3 +-- inc/lang/en/lang.php | 1 + lib/scripts/behaviour.js | 21 +++++++++++---------- lib/tpl/dokuwiki/css/_admin.less | 9 +++++++++ 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/inc/Ui/Admin.php b/inc/Ui/Admin.php index 27a57225c..d3bbc6503 100644 --- a/inc/Ui/Admin.php +++ b/inc/Ui/Admin.php @@ -84,8 +84,7 @@ class Admin extends Ui { if(substr($conf['savedir'], 0, 2) !== './') return; $img = DOKU_URL . $conf['savedir'] . '/dont-panic-if-you-see-this-in-your-logs-it-means-your-directory-permissions-are-correct.png'; - echo '⚠'; + echo '
'; } /** diff --git a/inc/lang/en/lang.php b/inc/lang/en/lang.php index 000368ac2..9e56d6f84 100644 --- a/inc/lang/en/lang.php +++ b/inc/lang/en/lang.php @@ -171,6 +171,7 @@ $lang['js']['media_done_btn'] = 'Done'; $lang['js']['media_drop'] = 'Drop files here to upload'; $lang['js']['media_cancel'] = 'remove'; $lang['js']['media_overwrt'] = 'Overwrite existing files'; +$lang['js']['data_insecure'] = 'WARNING: It seems your data directory is not properly secured. Please read about Web Access Security in DokuWiki.'; $lang['rssfailed'] = 'An error occurred while fetching this feed: '; $lang['nothingfound'] = 'Nothing was found.'; diff --git a/lib/scripts/behaviour.js b/lib/scripts/behaviour.js index 009081f80..608a29de0 100644 --- a/lib/scripts/behaviour.js +++ b/lib/scripts/behaviour.js @@ -215,25 +215,26 @@ var dw_behaviour = { * avoids overeager security bans (see #3363) */ securityCheck: function () { - var $checkA = jQuery('#security__check'); - if (!$checkA.length) return; - if (sessionStorage.getItem('security-check')) { + var $checkDiv = jQuery('#security__check'); + if (!$checkDiv.length) return; + if (sessionStorage.getItem('dw-security-check:' + DOKU_BASE)) { // check was already executed successfully - $checkA.remove(); + $checkDiv.remove(); return; } - var img = new Image(347, 63); + var img = new Image(); img.onerror = function () { // successful check will not be repeated during session - $checkA.remove(); - sessionStorage.setItem('security-check', true); + $checkDiv.remove(); + sessionStorage.setItem('dw-security-check:' + DOKU_BASE, true); } img.onload = function () { - // check failed, display the image - $checkA.html(img); + // check failed, display a warning message + $checkDiv.html(LANG.data_insecure); + $checkDiv.addClass('error'); } - img.src = $checkA.data('src'); + img.src = $checkDiv.data('src'); } }; diff --git a/lib/tpl/dokuwiki/css/_admin.less b/lib/tpl/dokuwiki/css/_admin.less index 38ca4bc63..115861203 100644 --- a/lib/tpl/dokuwiki/css/_admin.less +++ b/lib/tpl/dokuwiki/css/_admin.less @@ -61,4 +61,13 @@ clear: right; float: left; } + + /* data directory security check */ + #security__check { + float: right; + max-width: 20em; + } + [dir=rtl] & #admin__version { + float: left; + } } From b68a7721c62562cbcd3924f5a14b4df85b5b2305 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 11 Jan 2021 17:23:22 +0100 Subject: [PATCH 3/3] add cache buster on security check request When doing a normal reload on the admin page to recheck the security, the browser may not re-request the image again. This adds a timestamp to the URL to ensure the image is tested again --- lib/scripts/behaviour.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/scripts/behaviour.js b/lib/scripts/behaviour.js index 608a29de0..41702fad7 100644 --- a/lib/scripts/behaviour.js +++ b/lib/scripts/behaviour.js @@ -234,7 +234,7 @@ var dw_behaviour = { $checkDiv.html(LANG.data_insecure); $checkDiv.addClass('error'); } - img.src = $checkDiv.data('src'); + img.src = $checkDiv.data('src') + '?t=' + Date.now(); } };