From 00ee91837faf4807fb17dde3272ca8248a9dcd94 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 5 Oct 2009 14:04:27 -0700 Subject: Convert direct lookups for the user table using ORM to using the user::lookup_by_name and user_lookup API methods. Convert the Admin_User controller Convert the login and password change controller Change the item model to call user::lookup to get the owner. On the log model, delete the relationship between the log and user table, and replace with a call to user::lookup (cherry picked from commit 194cc3b27a73afe5119da9f09407c1e068dc6fa3) Create the get_user_list, lookup_by_name, lookup_by_hash and get_group_list api functions --- modules/gallery/models/item.php | 2 +- modules/gallery/models/log.php | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 6499fd2d..6f0e3525 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -333,7 +333,7 @@ class Item_Model extends ORM_MPTT { // This relationship depends on an outside module, which may not be present so handle // failures gracefully. try { - return model_cache::get("user", $this->owner_id); + return user::lookup($this->owner_id); } catch (Exception $e) { return null; } diff --git a/modules/gallery/models/log.php b/modules/gallery/models/log.php index 6734afb8..d143d7bd 100644 --- a/modules/gallery/models/log.php +++ b/modules/gallery/models/log.php @@ -18,5 +18,20 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class Log_Model extends ORM { - protected $has_one = array("user"); + /** + * @see ORM::__get() + */ + public function __get($column) { + if ($column == "user") { + // This relationship depends on an outside module, which may not be present so handle + // failures gracefully. + try { + return user::lookup($this->user_id); + } catch (Exception $e) { + return null; + } + } else { + return parent::__get($column); + } + } } -- cgit v1.2.3 From 79b4b8bdc606ecd46b258e32b3973511a43d38b0 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Fri, 9 Oct 2009 01:25:03 -0700 Subject: update the Access_Helper_Test to use the user::lookup_by_name API method. --- modules/gallery/tests/Access_Helper_Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index 59cec453..72d7e04c 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -33,7 +33,7 @@ class Access_Helper_Test extends Unit_Test_Case { } catch (Exception $e) { } try { - $user = ORM::factory("user")->where("name", "access_test")->find(); + $user = user::lookup_by_name("access_test"); if ($user->loaded) { $user->delete(); } @@ -307,7 +307,7 @@ class Access_Helper_Test extends Unit_Test_Case { $group->save(); access::allow($group, "edit", $root); - $user = ORM::factory("user", $user->id); // reload() does not flush related columns + $user = user::lookup($user->id); // reload() does not flush related columns user::set_active($user); // And verify that the user can edit. -- cgit v1.2.3 From 0a66ef9cc785fa5fb3614e7664c424d13ff09728 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 13 Oct 2009 10:36:50 -0700 Subject: Don't allow users to change the file extension of photos/movies If you can change the extension, then you can alter the way the server handles the file, which is a security problem. So for example, you can change a .JPG to a .PHP and then if you put some malicious PHP code in the EXIF data, you can get the server to execute it. Vulnerability is low because only users who have edit permissions could do this. Fixes ticket #846 --- modules/gallery/controllers/movies.php | 13 ++++++++++++- modules/gallery/controllers/photos.php | 12 +++++++++++- modules/gallery/helpers/movie.php | 3 ++- modules/gallery/helpers/photo.php | 3 ++- 4 files changed, 27 insertions(+), 4 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 2a917c58..01a9fc8b 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -61,7 +61,18 @@ class Movies_Controller extends Items_Controller { access::required("edit", $movie); $form = movie::get_edit_form($movie); - if ($valid = $form->validate()) { + $valid = $form->validate(); + + if ($valid) { + $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); + $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); + if (strcasecmp($new_ext, $old_ext)) { + $form->edit_item->filename->add_error("illegal_extension", 1); + $valid = false; + } + } + + if ($valid) { if ($form->edit_item->filename->value != $movie->name || $form->edit_item->slug->value != $movie->slug) { // Make sure that there's not a name or slug conflict diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 81e7519e..fbff53ce 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -63,7 +63,17 @@ class Photos_Controller extends Items_Controller { $form = photo::get_edit_form($photo); $valid = $form->validate(); - if ($valid = $form->validate()) { + + if ($valid) { + $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); + $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); + if (strcasecmp($new_ext, $old_ext)) { + $form->edit_item->filename->add_error("illegal_extension", 1); + $valid = false; + } + } + + if ($valid) { if ($form->edit_item->filename->value != $photo->name || $form->edit_item->slug->value != $photo->slug) { // Make sure that there's not a name or slug conflict diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 98419387..9ca28fe6 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -141,7 +141,8 @@ class movie_Core { ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The movie name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") - ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")); + ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")) + ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($movie->slug) ->callback("item::validate_url_safe") ->error_messages( diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 99f28753..6677ddc9 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -169,7 +169,8 @@ class photo_Core { ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The photo name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") - ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")); + ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")) + ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($photo->slug) ->callback("item::validate_url_safe") ->error_messages( -- cgit v1.2.3