Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I think that base64-encoded absolute server_filesystem path to the image file is the security hole (image.php using this path as imgp-parameter).
I propose to export only path, that relative to the brilliant gallery albums directory.
This path will be used in brilliant_gallery.module when we need to export it as part of the URL.
Then, relative path will be expanded to the absolute path in the image.php, using relative path value that was passed in base64-encoded imgp parameter.
For more security, the .htaccess file may be used to protect brilliant gallery albums directory from exterior HTTP access.
I thing, this is the critical issue.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 566178-module.diff | 3.4 KB | drpchris |
| #4 | 566178-image.diff | 1.68 KB | drpchris |
| #4 | 566178-functions.diff | 785 bytes | drpchris |
| #3 | 566178-module.diff | 3.4 KB | drpchris |
| #3 | 566178-image.diff | 1.68 KB | drpchris |
Comments
Comment #1
api87 CreditAttribution: api87 commentedAdditional hole:
Attacker can form any absolute path, so he can view any image on the site (and HTTP-access-protected too): he only must know image's filesystem path. And when we use relative path, we must also check symbols as "..", because attacker can form correct path like "photos/../../protected_img/img.jpg" which means that he again gains access to any folder of the site.
I'm sorry, my English is not good.
Comment #2
drpchris CreditAttribution: drpchris commentedthis patch changes path passed to image.php to be based on the galleries folder (ex:
/albums/albumname/file.jpg).image.php checks for any '..' in the path and exits if it finds them.
This patch also incorporates the fixes proposed in http://drupal.org/node/566184 which checks that the image width and height params are valid (and incorporates comment #4 to check that the numbers make relative sense).
Use this patch instead of what's in http://drupal.org/node/566184
Made against the development branch, but should work fine against 6.x-3.4
Please test this out!
Comment #3
drpchris CreditAttribution: drpchris commentedUpdated the diff files against the latest dev release.
Comment #4
drpchris CreditAttribution: drpchris commentedwhoops, did the diffs the wrong way
these diffs go together as a set
Comment #5
Vacilando CreditAttribution: Vacilando commented@api87; thanks for the reporting the findings.
@druchris1; huge thanks for the code. Since the dev changed a lot in the meantime, I had to apply it manually. Also, Picasa galleries did not work at all using the relative path encoding. Eventually I've made it compulsory to place the Picasa local cache into a subfolder of /files (just like the base BG gallery folder), and then fixed the paths, so things work now.
Committed to the dev version. If you say things work fine, I'll make a new version.
Comment #6
Vacilando CreditAttribution: Vacilando commentedI've found numerous more problems; the gallery management system was broken, cron cache cleaning as well, etc.
Hopefully fixed it all.
Committed to 6.x-3.6. One of the best BG versions ever, I believe / hope!