From 0a66ef9cc785fa5fb3614e7664c424d13ff09728 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
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')

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