Don't allow albums/photos/movies to end in "." because it risks

securit issues (and so Kohana won't route them, see
http://dev.kohanaphp.com/issues/684).

Partial fix for ticket #248.

6 files changed, 64 insertions, 5 deletions

diff --git a/core/helpers/album.php b/core/helpers/album.php
index c60527b2..1e0d1f7d 100644
--- a/core/helpers/album.php
+++ b/core/helpers/album.php
@@ -41,6 +41,12 @@ class album_Core {
       throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
     }
 
+    // We don't allow trailing periods as a security measure
+    // ref: http://dev.kohanaphp.com/issues/684
+    if (rtrim($name, ".") != $name) {
+      throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
+    }
+
     $album = ORM::factory("item");
     $album->type = "album";
     $album->title = $title;
diff --git a/core/helpers/movie.php b/core/helpers/movie.php
index 15ac554f..3293d4ac 100644
--- a/core/helpers/movie.php
+++ b/core/helpers/movie.php
@@ -43,6 +43,16 @@ class movie_Core {
       throw new Exception("@todo MISSING_MOVIE_FILE");
     }
 
+    if (strpos($name, "/")) {
+      throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
+    }
+
+    // We don't allow trailing periods as a security measure
+    // ref: http://dev.kohanaphp.com/issues/684
+    if (rtrim($name, ".") != $name) {
+      throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
+    }
+
     $movie_info = movie::getmoviesize($filename);
 
     // Force an extension onto the name
@@ -93,7 +103,7 @@ class movie_Core {
     graphics::generate($movie);
 
     // If the parent has no cover item, make this it.
-    if ($parent->album_cover_item_id == null)  {
+    if (access::can("edit", $parent) && $parent->album_cover_item_id == null)  {
       item::make_album_cover($movie);
     }
 
diff --git a/core/helpers/photo.php b/core/helpers/photo.php
index 0015bd99..8b0e1eab 100644
--- a/core/helpers/photo.php
+++ b/core/helpers/photo.php
@@ -47,6 +47,12 @@ class photo_Core {
       throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
     }
 
+    // We don't allow trailing periods as a security measure
+    // ref: http://dev.kohanaphp.com/issues/684
+    if (rtrim($name, ".") != $name) {
+      throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
+    }
+
     $image_info = getimagesize($filename);
 
     // Force an extension onto the name
diff --git a/core/tests/Album_Helper_Test.php b/core/tests/Album_Helper_Test.php
index 522d58d9..80afa8d1 100644
--- a/core/tests/Album_Helper_Test.php
+++ b/core/tests/Album_Helper_Test.php
@@ -71,4 +71,17 @@ class Album_Helper_Test extends Unit_Test_Case {
 
     $this->assert_true(false, "Shouldn't create an album with / in the name");
   }
+
+  public function create_album_silently_trims_trailing_periods_test() {
+    $rand = rand();
+    $root = ORM::factory("item", 1);
+    try {
+      $album = album::create($root, $rand . "..", $rand, $rand);
+    } catch (Exception $e) {
+      $this->assert_equal("@todo NAME_CANNOT_END_IN_PERIOD", $e->getMessage());
+      return;
+    }
+
+    $this->assert_true(false, "Shouldn't create an album with trailing . in the name");
+  }
 }
diff --git a/core/tests/Movie_Helper_Test.php b/core/tests/Movie_Helper_Test.php
index 0899154e..b92ef3f8 100644
--- a/core/tests/Movie_Helper_Test.php
+++ b/core/tests/Movie_Helper_Test.php
@@ -22,8 +22,7 @@ class Movie_Helper_Test extends Unit_Test_Case {
     $rand = rand();
     $root = ORM::factory("item", 1);
     try {
-      $filename = DOCROOT . "core/tests/test.jpg";
-      $photo = photo::create($root, $filename, "$rand/.jpg", $rand, $rand);
+      $movie = movie::create($root, DOCROOT . "core/tests/test.jpg", "$rand/.jpg", $rand, $rand);
     } catch (Exception $e) {
       // pass
       return;
@@ -31,4 +30,17 @@ class Movie_Helper_Test extends Unit_Test_Case {
 
     $this->assert_true(false, "Shouldn't create a movie with / in the name");
   }
+
+  public function create_movie_shouldnt_allow_names_with_trailing_periods_test() {
+    $rand = rand();
+    $root = ORM::factory("item", 1);
+    try {
+      $movie = movie::create($root, DOCROOT . "core/tests/test.jpg", "$rand.jpg.", $rand, $rand);
+    } catch (Exception $e) {
+      $this->assert_equal("@todo NAME_CANNOT_END_IN_PERIOD", $e->getMessage());
+      return;
+    }
+
+    $this->assert_true(false, "Shouldn't create a movie with trailing . in the name");
+  }
 }
diff --git a/core/tests/Photo_Helper_Test.php b/core/tests/Photo_Helper_Test.php
index 81405b79..deb11bb9 100644
--- a/core/tests/Photo_Helper_Test.php
+++ b/core/tests/Photo_Helper_Test.php
@@ -85,8 +85,7 @@ class Photo_Helper_Test extends Unit_Test_Case {
     $rand = rand();
     $root = ORM::factory("item", 1);
     try {
-      $filename = DOCROOT . "core/tests/test.jpg";
-      $photo = photo::create($root, $filename, "$rand/.jpg", $rand, $rand);
+      $photo = photo::create($root, DOCROOT . "core/tests/test.jpg", "$rand/.jpg", $rand, $rand);
     } catch (Exception $e) {
       // pass
       return;
@@ -94,4 +93,17 @@ class Photo_Helper_Test extends Unit_Test_Case {
 
     $this->assert_true(false, "Shouldn't create a photo with / in the name");
   }
+
+  public function create_photo_silently_trims_trailing_periods_test() {
+    $rand = rand();
+    $root = ORM::factory("item", 1);
+    try {
+      $photo = photo::create($root, DOCROOT . "core/tests/test.jpg", "$rand.jpg.", $rand, $rand);
+    } catch (Exception $e) {
+      $this->assert_equal("@todo NAME_CANNOT_END_IN_PERIOD", $e->getMessage());
+      return;
+    }
+
+    $this->assert_true(false, "Shouldn't create a photo with trailing . in the name");
+  }
 }