mirror of
https://github.com/Piwigo/Piwigo.git
synced 2026-11-03 07:22:22 +01:00
[authentication] URL protection for derivative images/item, not only for original image #261
Assignees
adam (Adam Wojcieszonek)
Clear assignees
adam (Adam Wojcieszonek)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: adam/Piwigo#261
Reference in new issue
Block a user
Reported by benhup on 27 Jun 2014 15:36
Version: 2.6.2
URLS from original images/files are protected when enabling in /piwigo/local/config/config.inc.php:
$conf['original_url_protection'] = 'images';
Derivative/resized images are NOT URL protected.
When a user logs out of Piwigo, the URLs to display images when a user was logged in, are still accissible. This is due to the fact that an URL is used that directly point to the image itself.
The situation is insecure, allowing anyone to access images, without being logged in.
Steps to reproduce: 1. use Piwigo 2.6.3 (or any version tha is 2.4+)
2. Place images/files in /piwigo/galleries/
3. Log in to Piwigo
4. Synchronize and allow Piwigo to create an album out of the files in /piwigo/galleries/
5. open an album or an indiviual image
6. Open the source or page info from the webpage.
7. The plain URLs are shown, pointing directly to the image location.
This counts for all sizes/all derivatives.
Additional information: First efforts are made under:
http://piwigo.org/forum/viewtopic.php?id=24077
and
http://piwigo.org/forum/viewtopic.php?id=24090
Piwigo Bugtracker #3096
Comment posted by benhup on 28 Jun 2014 11:05
--- D:/piwigo-2.6.3/piwigo/action.php Sun Jan 05 01:19:26 2014
+++ D:/piwigo-2.6.3/piwigo/benchanges/action.php Sat Jun 28 12:52:30 2014
@@ -61,7 +61,7 @@
if (!isset($_GET['id'])
or !is_numeric($_GET['id'])
or !isset($_GET['part'])
{
do_error(400, 'Invalid request - id/part');
}
@@ -100,6 +100,7 @@
include_once(PHPWG_ROOT_PATH.'include/functions_picture.inc.php');
$file='';
+$part = $_GET['part'];
switch ($_GET['part'])
{
case 'e':
@@ -116,6 +117,29 @@
case 'r':
$file = original_to_representative( get_element_path($element_info), $element_info['representative_ext'] );
break;
}
if ( empty($file) )
Comment posted by benhup on 28 Jun 2014 11:05
--- D:/piwigo-2.6.3/piwigo/include/derivative.inc.php Sun Jan 05 01:19:26 2014
+++ D:/piwigo-2.6.3/piwigo/benchanges/derivative.inc.php Sat Jun 28 13:04:02 2014
@@ -383,7 +383,14 @@
}
else
{
}
Comment posted by benhup on 28 Jun 2014 11:06
--- D:/piwigo-2.6.3/piwigo/i.php Sat Jun 28 12:57:57 2014
+++ D:/piwigo-2.6.3/piwigo/benchanges/i.php Sat Jun 28 12:59:21 2014
@@ -349,7 +349,7 @@
return false;
}
-function send_derivative($expires)
+function send_derivative($expires, $id = null)
{
global $page;
@@ -358,7 +358,14 @@
include_once(PHPWG_ROOT_PATH.'include/functions_cookie.inc.php');
include_once(PHPWG_ROOT_PATH.'include/functions_url.inc.php');
return;
}
$fp = fopen($page['derivative_path'], 'rb');
@@ -446,6 +453,11 @@
if (!$need_generate)
{
if ( isset( $_SERVER['HTTP_IF_MODIFIED_SINCE'] )
and strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']) == $derivative_mtime)
{// send the last mod time of the file back
@@ -457,6 +469,8 @@
exit;
}
+$id = '';
+
include_once(PHPWG_ROOT_PATH . 'admin/include/image.class.php');
$page['coi'] = null;
if (strpos($page['src_location'], '/pwg_representative/')===false
@@ -473,6 +487,7 @@
{
$page['original_size'] = array($row['width'],$row['height']);
@@ -618,7 +633,7 @@
@chmod($page['derivative_path'], 0644);
$timing['save'] = time_step($step);
-send_derivative($expires);
+send_derivative($expires,$id);
$timing['send'] = time_step($step);
ilog('perf',
Comment posted by tho on 4 Sep 2014 20:59
Hi,
I came up with a different implementation, using the 'derivative_url_style == 2' configuration (script). I only did a few quick tests atm, and there are a few glitches to fix, but I wanted to get your feedback first on the global approach.
The idea is to pass a session variable to the i.php script, that encodes the path to the derivative file and that is set by the top level page generating the link. This way, a user can only see what the top level page has generated for him. Of course, to be effective, the _data/i directory also has to be protected by a 'deny all' apache directive (.htaccess or httpd.conf).
Here is the diff. What do you think?
diff --git i.php i.php
index 8ca3b68..5771eb6 100644
--- i.php
+++ i.php
@@ -201,6 +201,11 @@ function parse_request()
$req = ltrim($req, '/');
foreach (preg_split('#/+#', $req) as $token)
{
preg_match($conf['sync_chars_regex'], $token) or ierror('Invalid chars in r
@@ -393,6 +398,9 @@ foreach( explode(',','load,rotate,crop,scale,sharpen,waterma
$timing[$k] = '';
}
+include_once(PHPWG_ROOT_PATH .'/include/constants.php');
+include_once(PHPWG_ROOT_PATH .'/include/functions_cookie.inc.php');
+include_once(PHPWG_ROOT_PATH .'/include/functions_session.inc.php');
include_once(PHPWG_ROOT_PATH .'include/dblayer/functions_'.$conf['dblayer'].'.i
include_once( PHPWG_ROOT_PATH .'/include/derivative_params.inc.php');
include_once( PHPWG_ROOT_PATH .'/include/derivative_std_params.inc.php');
@@ -411,7 +419,7 @@ pwg_db_check_charset();
list($conf['derivatives']) = pwg_db_fetch_row(pwg_query('SELECT value FROM '.$p
ImageStdParams::load_from_db();
+session_start();
parse_request();
//var_export($page);
diff --git include/derivative.inc.php include/derivative.inc.php
index 49fa9d0..511b79a 100644
--- include/derivative.inc.php
+++ include/derivative.inc.php
@@ -379,7 +379,10 @@ final class DerivativeImage
$rel_url = 'i';
if ($conf['php_extension_in_urls']) $rel_url .= '.php';
if ($conf['question_mark_in_urls']) $rel_url .= '?';
else
{
Comment posted by benhup on 16 Mar 2015 22:03
Hi Tho,
Unfortunately your e-mail is not shown in your profile.
Please contact me. I have tested your solution. The small amount of code is great. One thing that I also bumped into is that once the smaller size file is genereated, next access: a link that reveals the true image name and enables direct access again is used with i.php.
Example: first time access to image shows this URL: http://host/piwigo-2.7.4/i.php?/YWQ3MGQ1YjQyOTJmZGU1ZThhNmRhNWM0ODllMjk0NDY=
Now change folder and go back to the image, select the same size. The URL now shown is: http://host/piwigo-2.7.4/_data/i/galleries/ben1/image2-2s.jpg
Also tested: when logged out file is not accessible via URL anymore = good.
Used i.php URL is not valid after logging in again = could be a nagging problem if people want to share a link quickly. When person A copy/pastes a link via mail and sends the link to person B. Person B logs in and copy/pastes the link in browser. Current behaviour in your solution: access denied.
Great step, lets see if we can build on this. I'd like to hear from you. Best, Ben
Comment posted by benhup on 16 Mar 2015 22:42
Hi Tho,
Correction:
When /config/config.inc.php contains:
$conf['original_url_protection'] = 'images'; /* mask URL of original image */
$conf['derivative_url_style'] = 2; /* mask URL of resized images */
All appears to be well with preliminary testing.
I mean: after a resized image is generated at first time access, you leave the page and return to the image and select the same size. The URLs are again encrypted.
Logging out results in all files not being accessible = good.
Logging out and logging back in again results in:
Original file accessible again via URL e.g.: http://host/piwigo-2.7.4/action.php?id=5&part=e
Resized image URLs remain unusable, even after logging in .e.g.: http://host/piwigo-2.7.4/i.php?/ZmU5YzU4MTYxODQ4Nzc1NGJkYzRlMzUxMmEyNTBiYzE=
But: when you access the image via Piwigo and refresh the image with the base64_encoded URL in your open tab of your browser, it reappears again. This is because the $key van befound in the session again and thus the $loc.
By preliminary testing I also mean: I have not tested this by accessing large amounts of files and what happens when logged in for a long period of time. Also I have to think if any of these changes create security vulnerabilities.
Anyway, whats your reflection?
Best,
Ben
The patch merged to current master is attached.
patch.diff.zip