Fix a bunch of XSS vulnerabilities turned up by manual inspection

using the checklist in ticket #385.

15 files changed, 48 insertions, 34 deletions

diff --git a/modules/gallery/controllers/admin_advanced_settings.php b/modules/gallery/controllers/admin_advanced_settings.php
index 79bc1183..64007fdb 100644
--- a/modules/gallery/controllers/admin_advanced_settings.php
+++ b/modules/gallery/controllers/admin_advanced_settings.php
@@ -46,7 +46,7 @@ class Admin_Advanced_Settings_Controller extends Admin_Controller {
     module::set_var($module_name, $var_name, Input::instance()->post("value"));
     message::success(
       t("Saved value for %var (%module_name)",
-        array("var" => $var_name, "module_name" => $module_name)));
+        array("var" => p::clean($var_name), "module_name" => $module_name)));
 
     print json_encode(array("result" => "success"));
   }
diff --git a/modules/gallery/controllers/admin_modules.php b/modules/gallery/controllers/admin_modules.php
index ed1f7665..dfa49a0e 100644
--- a/modules/gallery/controllers/admin_modules.php
+++ b/modules/gallery/controllers/admin_modules.php
@@ -56,7 +56,7 @@ class Admin_Modules_Controller extends Admin_Controller {
 
     module::event("module_change", $changes);
 
-    // @todo this type of collation is questionable from a i18n perspective
+    // @todo this type of collation is questionable from an i18n perspective
     if ($activated_names) {
       message::success(t("Activated: %names", array("names" => join(", ", $activated_names))));
     }
diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index 22f50fb8..d141d157 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -111,7 +111,8 @@ class Albums_Controller extends Items_Controller {
 
       log::success("content", "Created an album",
                html::anchor("albums/$new_album->id", "view album"));
-      message::success(t("Created album %album_title", array("album_title" => $new_album->title)));
+      message::success(
+        t("Created album %album_title", array("album_title" => p::clean($new_album->title))));
 
       print json_encode(
         array("result" => "success",
@@ -143,7 +144,8 @@ class Albums_Controller extends Items_Controller {
         user::active()->id);
 
       log::success("content", "Added a photo", html::anchor("photos/$photo->id", "view photo"));
-      message::success(t("Added photo %photo_title", array("photo_title" => $photo->title)));
+      message::success(
+        t("Added photo %photo_title", array("photo_title" => p::clean($photo->title))));
 
       print json_encode(
         array("result" => "success",
@@ -197,7 +199,8 @@ class Albums_Controller extends Items_Controller {
       module::event("item_updated", $orig, $album);
 
       log::success("content", "Updated album", "<a href=\"albums/$album->id\">view</a>");
-      message::success(t("Saved album %album_title", array("album_title" => $album->title)));
+      message::success(
+        t("Saved album %album_title", array("album_title" => p::clean($album->title))));
 
       print json_encode(
         array("result" => "success",
diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php
index d8cca825..30a5d78c 100644
--- a/modules/gallery/controllers/movies.php
+++ b/modules/gallery/controllers/movies.php
@@ -94,7 +94,8 @@ class Movies_Controller extends Items_Controller {
       module::event("item_updated", $orig, $photo);
 
       log::success("content", "Updated photo", "<a href=\"photos/$photo->id\">view</a>");
-      message::success(t("Saved photo %photo_title", array("photo_title" => $photo->title)));
+      message::success(
+        t("Saved photo %photo_title", array("photo_title" => p::clean($photo->title))));
 
       print json_encode(
         array("result" => "success",
diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index f5be5d59..6a62e859 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -87,7 +87,8 @@ class Photos_Controller extends Items_Controller {
       module::event("item_updated", $orig, $photo);
 
       log::success("content", "Updated photo", "<a href=\"photos/$photo->id\">view</a>");
-      message::success(t("Saved photo %photo_title", array("photo_title" => $photo->title)));
+      message::success(
+        t("Saved photo %photo_title", array("photo_title" => p::clean($photo->title))));
 
       print json_encode(
         array("result" => "success",
diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php
index e89d9701..5d3d8885 100644
--- a/modules/gallery/controllers/quick.php
+++ b/modules/gallery/controllers/quick.php
@@ -89,7 +89,7 @@ class Quick_Controller extends Controller {
     access::required("view", $item->parent());
     access::required("edit", $item->parent());
 
-    $msg = t("Made <b>%title</b> this album's cover", array("title" => $item->title));
+    $msg = t("Made <b>%title</b> this album's cover", array("title" => p::clean($item->title)));
 
     item::make_album_cover($item);
     message::success($msg);
@@ -105,9 +105,10 @@ class Quick_Controller extends Controller {
     if ($item->is_album()) {
       print t(
         "Delete the album <b>%title</b>? All photos and movies in the album will also be deleted.",
-        array("title" => $item->title));
+        array("title" => p::clean($item->title)));
     } else {
-      print t("Are you sure you want to delete <b>%title</b>?", array("title" => $item->title));
+      print t("Are you sure you want to delete <b>%title</b>?",
+              array("title" => p::clean($item->title)));
     }
 
     $form = item::get_delete_form($item);
@@ -121,9 +122,9 @@ class Quick_Controller extends Controller {
     access::required("edit", $item);
 
     if ($item->is_album()) {
-      $msg = t("Deleted album <b>%title</b>", array("title" => $item->title));
+      $msg = t("Deleted album <b>%title</b>", array("title" => p::clean($item->title)));
     } else {
-      $msg = t("Deleted photo <b>%title</b>", array("title" => $item->title));
+      $msg = t("Deleted photo <b>%title</b>", array("title" => p::clean($item->title)));
     }
 
     $item->delete();
diff --git a/modules/gallery/helpers/l10n_client.php b/modules/gallery/helpers/l10n_client.php
index 20f81ecc..e153532c 100644
--- a/modules/gallery/helpers/l10n_client.php
+++ b/modules/gallery/helpers/l10n_client.php
@@ -112,7 +112,9 @@ class l10n_client_Core {
     //    {key:<key_2>, ...}
     //   ]
     $count = count($response);
-    log::info("translations", "Installed $count new / updated translation messages");
+    log::info("translations",
+              t2("Installed 1 new / updated translation message",
+                 "Installed %count new / updated translation messages", $count));
 
     foreach ($response as $message_data) {
       // @todo Better input validation
diff --git a/modules/organize/controllers/organize.php b/modules/organize/controllers/organize.php
index 57709cb5..5f80805c 100644
--- a/modules/organize/controllers/organize.php
+++ b/modules/organize/controllers/organize.php
@@ -283,10 +283,10 @@ class Organize_Controller extends Controller {
 
       if ($item->is_album()) {
         log::success("content", "Updated album", "<a href=\"albums/$item->id\">view</a>");
-        $message = t("Saved album %album_title", array("album_title" => $item->title));
+        $message = t("Saved album %album_title", array("album_title" => p::clean($item->title)));
       } else {
         log::success("content", "Updated photo", "<a href=\"photos/$item->id\">view</a>");
-        $message = t("Saved photo %photo_title", array("photo_title" => $item->title));
+        $message = t("Saved photo %photo_title", array("photo_title" => p::clean($item->title)));
       }
       print json_encode(array("form" => $form->__toString(), "message" => $message));
     } else {
@@ -325,7 +325,7 @@ class Organize_Controller extends Controller {
       module::event("item_updated", $orig, $item);
 
       log::success("content", "Updated album", "<a href=\"albums/$item->id\">view</a>");
-      $message = t("Saved album %album_title", array("album_title" => $item->title));
+      $message = t("Saved album %album_title", array("album_title" => p::clean($item->title)));
       print json_encode(array("form" => $form->__toString(), "message" => $message));
     } else {
       print json_encode(array("form" => $form->__toString()));
diff --git a/modules/server_add/controllers/admin_server_add.php b/modules/server_add/controllers/admin_server_add.php
index a3f9aa96..a30215b8 100644
--- a/modules/server_add/controllers/admin_server_add.php
+++ b/modules/server_add/controllers/admin_server_add.php
@@ -40,7 +40,7 @@ class Admin_Server_Add_Controller extends Admin_Controller {
         module::set_var("server_add", "authorized_paths", serialize($paths));
         $form->add_path->inputs->path->value = "";
 
-        message::success(t("Added path %path", array("path" => $path)));
+        message::success(t("Added path %path", array("path" => p::clean($path))));
 
         server_add::check_config($paths);
         url::redirect("admin/server_add");
@@ -62,7 +62,7 @@ class Admin_Server_Add_Controller extends Admin_Controller {
     $path = $this->input->get("path");
     $paths = unserialize(module::get_var("server_add", "authorized_paths"));
     unset($paths[$path]);
-    message::success(t("Removed path %path", array("path" => $path)));
+    message::success(t("Removed path %path", array("path" => p::clean($path))));
     module::set_var("server_add", "authorized_paths", serialize($paths));
     server_add::check_config($paths);
 
diff --git a/modules/server_add/controllers/server_add.php b/modules/server_add/controllers/server_add.php
index c92b4f7e..05ea5058 100644
--- a/modules/server_add/controllers/server_add.php
+++ b/modules/server_add/controllers/server_add.php
@@ -110,7 +110,7 @@ class Server_Add_Controller extends Controller {
                               "url" => "",
                               "task" => array(
                                 "id" => -1, "done" => 1, "percent_complete" => 100,
-                                "status" => t("No Eligible files, import cancelled"))));
+                                "status" => t("No eligible files, import cancelled"))));
       return;
     }
 
diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php
index af5055ff..dcdc16b9 100644
--- a/modules/tag/controllers/admin_tags.php
+++ b/modules/tag/controllers/admin_tags.php
@@ -53,8 +53,8 @@ class Admin_Tags_Controller extends Admin_Controller {
       $name = $tag->name;
       Database::instance()->delete("items_tags", array("tag_id" => "$tag->id"));
       $tag->delete();
-      message::success(t("Deleted tag %tag_name", array("tag_name" => $name)));
-      log::success("tags", t("Deleted tag %tag_name", array("tag_name" => $name)));
+      message::success(t("Deleted tag %tag_name", array("tag_name" => p::clean($name))));
+      log::success("tags", t("Deleted tag %tag_name", array("tag_name" => p::clean($name))));
 
       print json_encode(
         array("result" => "success",
@@ -98,7 +98,7 @@ class Admin_Tags_Controller extends Admin_Controller {
       $tag->save();
 
       $message = t("Renamed tag %old_name to %new_name",
-                   array("old_name" => $old_name, "new_name" => $tag->name));
+                   array("old_name" => p::clean($old_name), "new_name" => p::clean($tag->name)));
       message::success($message);
       log::success("tags", $message);
 
diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index fe8061aa..b5dc6cb5 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -50,7 +50,7 @@ class Admin_Users_Controller extends Controller {
       }
 
       $user->save();
-      message::success(t("Created user %user_name", array("user_name" => $user->name)));
+      message::success(t("Created user %user_name", array("user_name" => p::clean($user->name))));
       print json_encode(array("result" => "success"));
     } else {
       print json_encode(array("result" => "error",
@@ -83,7 +83,7 @@ class Admin_Users_Controller extends Controller {
                               "form" => $form->__toString()));
     }
 
-    $message = t("Deleted user %user_name", array("user_name" => $name));
+    $message = t("Deleted user %user_name", array("user_name" => p::clean($name)));
     log::success("user", $message);
     message::success($message);
     print json_encode(array("result" => "success"));
@@ -139,7 +139,7 @@ class Admin_Users_Controller extends Controller {
       }
       $user->save();
 
-      message::success(t("Changed user %user_name", array("user_name" => $user->name)));
+      message::success(t("Changed user %user_name", array("user_name" => p::clean($user->name))));
       print json_encode(array("result" => "success"));
     } else {
       print json_encode(array("result" => "error",
@@ -200,7 +200,8 @@ class Admin_Users_Controller extends Controller {
     if ($valid) {
       $group = group::create($new_name);
       $group->save();
-      message::success(t("Created group %group_name", array("group_name" => $group->name)));
+      message::success(
+        t("Created group %group_name", array("group_name" => p::clean($group->name))));
       print json_encode(array("result" => "success"));
     } else {
       print json_encode(array("result" => "error",
@@ -229,7 +230,7 @@ class Admin_Users_Controller extends Controller {
                               "form" => $form->__toString()));
     }
 
-    $message = t("Deleted group %group_name", array("group_name" => $name));
+    $message = t("Deleted group %group_name", array("group_name" => p::clean($name)));
     log::success("group", $message);
     message::success($message);
     print json_encode(array("result" => "success"));
@@ -266,10 +267,12 @@ class Admin_Users_Controller extends Controller {
     if ($valid) {
       $group->name = $form->edit_group->inputs["name"]->value;
       $group->save();
-      message::success(t("Changed group %group_name", array("group_name" => $group->name)));
+      message::success(
+        t("Changed group %group_name", array("group_name" => p::clean($group->name))));
       print json_encode(array("result" => "success"));
     } else {
-      message::error(t("Failed to change group %group_name", array("group_name" => $group->name)));
+      message::error(
+        t("Failed to change group %group_name", array("group_name" => p::clean($group->name))));
       print json_encode(array("result" => "error",
                               "form" => $form->__toString()));
     }
diff --git a/modules/user/controllers/login.php b/modules/user/controllers/login.php
index 54a7905e..4d901051 100644
--- a/modules/user/controllers/login.php
+++ b/modules/user/controllers/login.php
@@ -62,7 +62,8 @@ class Login_Controller extends Controller {
       if (!$user->loaded || !user::is_correct_password($user, $form->login->password->value)) {
         log::warning(
           "user",
-          t("Failed login for %name", array("name" => $form->login->inputs["name"]->value)));
+          t("Failed login for %name",
+            array("name" => p::clean($form->login->inputs["name"]->value))));
         $form->login->inputs["name"]->add_error("invalid_login", 1);
         $valid = false;
       }
@@ -70,7 +71,7 @@ class Login_Controller extends Controller {
 
     if ($valid) {
       user::login($user);
-      log::info("user", t("User %name logged in", array("name" => $user->name)));
+      log::info("user", t("User %name logged in", array("name" => p::clean($user->name))));
     }
 
     // Either way, regenerate the session id to avoid session trapping
diff --git a/modules/user/controllers/logout.php b/modules/user/controllers/logout.php
index a541ed9b..63971789 100644
--- a/modules/user/controllers/logout.php
+++ b/modules/user/controllers/logout.php
@@ -23,8 +23,8 @@ class Logout_Controller extends Controller {
 
     $user = user::active();
     user::logout();
-    log::info("user", t("User %name logged out", array("name" => $user->name)),
-              html::anchor("user/$user->id", $user->name));
+    log::info("user", t("User %name logged out", array("name" => p::clean($user->name))),
+              html::anchor("user/$user->id", p::clean($user->name)));
     if ($this->input->get("continue")) {
       $item = url::get_item_from_uri($this->input->get("continue"));
       if (access::can("view", $item)) {
diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index 2dde11b8..ed3b9736 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -72,7 +72,9 @@ class Password_Controller extends Controller {
         ->message($message->render())
         ->send();
 
-      log::success("user", "Password reset email sent for user $user->name");
+      log::success(
+        "user",
+        t("Password reset email sent for user %name", array("name" => p::clean($user->name)));
     } else {
       // Don't include the username here until you're sure that it's XSS safe
       log::warning(