Fix a bunch of XSS vulnerabilities turned up by manual inspection

using the checklist in ticket #385.
This commit is contained in:
Bharat Mediratta
2009-07-01 17:57:39 -07:00
parent 7b1e3c5021
commit 8f9a943f55
15 changed files with 48 additions and 34 deletions

View File

@@ -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"));
}

View File

@@ -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))));
}

View File

@@ -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",

View File

@@ -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",

View File

@@ -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",

View File

@@ -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();

View File

@@ -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

View File

@@ -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()));

View File

@@ -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);

View File

@@ -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;
}

View File

@@ -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);

View File

@@ -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()));
}

View File

@@ -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

View File

@@ -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)) {

View File

@@ -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(