Piwigo Bugtracker

Piwigo bug tracker has moved to Github

This bugtracker is kept to provide history on old issues.


View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003096Piwigoauthenticationpublic2014.06.27 17:362015.03.17 09:18
Reporterbenhup 
Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
StatusnewResolutionopen 
PlatformAllOSAllOS VersionAll
Product Version2.6.2 
Target VersionFixed in Version 
Summary0003096: URL protection for derivative images/item, not only for original image
DescriptionURLS 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 Reproduce1. 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 InformationFirst efforts are made under:
http://piwigo.org/forum/viewtopic.php?id=24077 [^]
and
http://piwigo.org/forum/viewtopic.php?id=24090 [^]
TagsNo tags attached.
browserany
Database engine and versionAny
PHP versionAny
Web serverAny Apache
Attached Files

- Relationships

-  Notes
(0007495)
benhup (reporter)
2014.06.28 13: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'])
- or !in_array($_GET['part'], array('e','r') ) )
+ or !in_array($_GET['part'], array('e','r','sq','th','2s','xs','sm','me','la','xl','xx') ) )
 {
   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;
+ case 'sq':
+ $deriv = new DerivativeImage(IMG_SQUARE, new SrcImage($element_info));
+ case 'th':
+ $deriv = new DerivativeImage(IMG_THUMB, new SrcImage($element_info));
+ case '2s':
+ $deriv = new DerivativeImage(IMG_XXSMALL, new SrcImage($element_info));
+ case 'xs':
+ $deriv = new DerivativeImage(IMG_XSMALL, new SrcImage($element_info));
+ case 'sm':
+ $deriv = new DerivativeImage(IMG_SMALL, new SrcImage($element_info));
+ case 'me':
+ $deriv = new DerivativeImage(IMG_MEDIUM, new SrcImage($element_info));
+ case 'la':
+ $deriv = new DerivativeImage(IMG_LARGE, new SrcImage($element_info));
+ case 'xl':
+ $deriv = new DerivativeImage(IMG_XLARGE, new SrcImage($element_info));
+ case 'xx':
+ $deriv = new DerivativeImage(IMG_XXLARGE, new SrcImage($element_info));
+ $file = get_element_path($element_info);
+ $file = './_data/i/'.$file;
+ $picture_exts = implode('|',$conf['picture_ext']);
+ $file = preg_replace('/\.('.$picture_exts.')$/i','-'.$part.'.${1}',$file);
+ break;
 }
 
 if ( empty($file) )
(0007496)
benhup (reporter)
2014.06.28 13: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
     {
- $rel_url = $rel_path;
+ if( !empty($conf['original_url_protection']) )
+ {
+ $rel_url = get_action_url($src->id, substr($params->type,0,2), false);
+ }
+ else
+ {
+ $rel_url = $rel_path;
+ }
     }
   }
(0007497)
benhup (reporter)
2014.06.28 13: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');
 
- echo json_encode( array( 'url'=>embellish_url(get_absolute_root_url().$page['derivative_path']) ) );
+ if( !empty($conf['original_url_protection']) )
+ {
+ echo str_replace('&', '&', json_encode( array( 'url'=>embellish_url(get_action_url($id, substr($page['derivative_type'],0,2), false)) ) ) );
+ }
+ else
+ {
+ echo json_encode( array( 'url'=>embellish_url(get_absolute_root_url().$page['derivative_path']) ) );
+ }
     return;
   }
   $fp = fopen($page['derivative_path'], 'rb');
@@ -446,6 +453,11 @@
 
 if (!$need_generate)
 {
+ if( !empty($conf['original_url_protection']) )
+ {// if original_url_protection is enabled, only file creation and first-time retrieval is allowed; after-first-time: access denied
+ //echo "Access denied...";
+ //exit;
+ }
   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 @@
 
     if ( ($row=pwg_db_fetch_assoc(pwg_query($query))) )
     {
+ $id = $row['id'];
       if (isset($row['width']))
       {
         $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',
(0007615)
tho (reporter)
2014.09.04 22: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, '/');
 
+ $req = pwg_get_session_var($req);
+ if ($req == null) {
+ ierror('Not found', 404);
+ }
+
   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 .= '?';
- $rel_url .= '/'.$loc;
+
+ $key = base64_encode(md5($loc));
+ pwg_set_session_var($key, $loc);
+ $rel_url .= '/'.$key;
     }
     else
     {
(0007784)
benhup (reporter)
2015.03.16 23: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
(0007785)
benhup (reporter)
2015.03.16 23:42
edited on: 2015.03.17 09:18

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


- Issue History
Date Modified Username Field Change
2014.06.27 17:36 benhup New Issue
2014.06.27 17:36 benhup browser => any
2014.06.27 17:36 benhup Database engine and version => Any
2014.06.27 17:36 benhup PHP version => Any
2014.06.27 17:36 benhup Web server => Any Apache
2014.06.28 13:05 benhup Note Added: 0007495
2014.06.28 13:05 benhup Note Added: 0007496
2014.06.28 13:06 benhup Note Added: 0007497
2014.09.04 22:59 tho Note Added: 0007615
2015.03.16 23:03 benhup Note Added: 0007784
2015.03.16 23:42 benhup Note Added: 0007785
2015.03.17 09:18 benhup Note Edited: 0007785 View Revisions


Copyright © 2000 - 2017 MantisBT Team
Contact
Powered by Mantis Bugtracker