Skip to content

Commit

Permalink
Merge pull request from GHSA-3xfh-h685-w25m
Browse files Browse the repository at this point in the history 
Validate POST requests with CSRF token
  • Loading branch information
@Alanaktion
Alanaktion committed Sep 10, 2021
2 parents 90215ae + ea02851 commit 4032f44
Show file tree
Hide file tree
Showing 37 changed files with 299 additions and 116 deletions.
4 changes: 3 additions & 1 deletion SECURITY.md
View file
Expand Up @@ -6,6 +6,8 @@ Since our team is small, and we don't update often, we only support the latest r

## Reporting a Vulnerability

If you find an issue with the security of Phproject, let me know personally via email at alan@phpizza.com. I'll do my best to get back to you within 48 hours, at least with an idea of when we can fix the issue. Major issues that involve significant changes to the application can take several weeks since no one works on this project full time, but we'll do what we can to fix things.
If you find an issue with the security of Phproject, you may report the vulnerability on [huntr.dev](https://huntr.dev/bounties/disclose), or let me know personally via email at alan@phpizza.com. If emailing, I recommend encrypting your communication with [my PGP public key](https://keybase.io/alanaktion/pgp_keys.asc).

I'll do my best to get back to you within 48 hours, at least with an idea of when we can fix the issue. Major issues that involve significant changes to the application can take several weeks since no one works on this project full time, but we'll do what we can to fix things.

If it's been more than a week and we haven't replied, or if we have replied but it's been a while and we haven't communicated or fixed the issue you found, let us know and we'll invite you to a private fork where we can collaborate on a fix.
8 changes: 8 additions & 0 deletions app/controller.php
View file
Expand Up @@ -91,4 +91,12 @@ public function now($time = true)
{
return $time ? date("Y-m-d H:i:s") : date("Y-m-d");
}

/**
* Validate the request's CSRF token, exiting if invalid
*/
protected function validateCsrf()
{
\Helper\Security::instance()->validateCsrfToken();
}
}
33 changes: 19 additions & 14 deletions app/controller/admin.php
View file
Expand Up @@ -22,6 +22,8 @@ public function index(\Base $f3)
$f3->set("menuitem", "admin");

if ($f3->get("POST.action") == "clearcache") {
$this->validateCsrf();

$cache = \Cache::instance();

// Clear configured cache
Expand Down Expand Up @@ -153,6 +155,7 @@ public function config(\Base $f3)
*/
public function config_post_saveattribute(\Base $f3)
{
$this->validateCsrf();
$this->_requireAdmin(\Model\User::RANK_SUPER);

$attribute = str_replace("-", ".", $f3->get("POST.attribute"));
Expand Down Expand Up @@ -304,6 +307,7 @@ public function user_new(\Base $f3)
*/
public function user_save(\Base $f3)
{
$this->validateCsrf();
$security = \Helper\Security::instance();
$user = new \Model\User();
$user_id = $f3->get("POST.user_id");
Expand Down Expand Up @@ -399,6 +403,7 @@ public function user_save(\Base $f3)
*/
public function user_delete(\Base $f3, array $params)
{
$this->validateCsrf();
$user = new \Model\User();
$user->load($params["id"]);
if (!$user->id) {
Expand Down Expand Up @@ -434,6 +439,7 @@ public function user_delete(\Base $f3, array $params)
*/
public function user_undelete(\Base $f3, array $params)
{
$this->validateCsrf();
$user = new \Model\User();
$user->load($params["id"]);
if (!$user->id) {
Expand Down Expand Up @@ -485,20 +491,15 @@ public function groups(\Base $f3)
*/
public function group_new(\Base $f3)
{
$f3->set("title", $f3->get("dict.groups"));

if ($f3->get("POST")) {
$group = new \Model\User();
$group->name = $f3->get("POST.name");
$group->username = \Web::instance()->slug($group->name);
$group->role = "group";
$group->task_color = sprintf("%02X%02X%02X", mt_rand(0, 0xFF), mt_rand(0, 0xFF), mt_rand(0, 0xFF));
$group->created_date = $this->now();
$group->save();
$f3->reroute("/admin/groups");
} else {
$f3->error(405);
}
$this->validateCsrf();
$group = new \Model\User();
$group->name = $f3->get("POST.name");
$group->username = \Web::instance()->slug($group->name);
$group->role = "group";
$group->task_color = sprintf("%02X%02X%02X", mt_rand(0, 0xFF), mt_rand(0, 0xFF), mt_rand(0, 0xFF));
$group->created_date = $this->now();
$group->save();
$f3->reroute("/admin/groups");
}

/**
Expand Down Expand Up @@ -532,6 +533,7 @@ public function group_edit(\Base $f3, array $params)
*/
public function group_delete(\Base $f3, array $params)
{
$this->validateCsrf();
$group = new \Model\User();
$group->load($params["id"]);
$group->delete();
Expand All @@ -549,6 +551,7 @@ public function group_delete(\Base $f3, array $params)
*/
public function group_ajax(\Base $f3)
{
$this->validateCsrf();
if (!$f3->get("AJAX")) {
$f3->error(400);
}
Expand Down Expand Up @@ -608,6 +611,7 @@ public function group_ajax(\Base $f3)
*/
public function group_setmanager(\Base $f3, array $params)
{
$this->validateCsrf();
$db = $f3->get("db.instance");

$group = new \Model\User();
Expand Down Expand Up @@ -645,6 +649,7 @@ public function sprints(\Base $f3)
*/
public function sprint_new(\Base $f3)
{
$this->validateCsrf();
$f3->set("title", $f3->get("dict.sprints"));

if ($post = $f3->get("POST")) {
Expand Down
3 changes: 3 additions & 0 deletions app/controller/backlog.php
View file
Expand Up @@ -141,6 +141,8 @@ public function redirect(\Base $f3)
*/
public function edit($f3)
{
$this->validateCsrf();

// Move project
$post = $f3->get("POST");
$issue = new \Model\Issue();
Expand All @@ -166,6 +168,7 @@ public function edit($f3)
*/
public function sort($f3)
{
$this->validateCsrf();
$this->_requireLogin(\Model\User::RANK_MANAGER);
$backlog = new \Model\Issue\Backlog();
if ($f3->get("POST.sprint_id")) {
Expand Down
12 changes: 11 additions & 1 deletion app/controller/index.php
View file
Expand Up @@ -68,6 +68,7 @@ public function login($f3)
*/
public function loginpost($f3)
{
$this->validateCsrf();
$user = new \Model\User();

// Load user by username or email address
Expand Down Expand Up @@ -111,6 +112,7 @@ public function loginpost($f3)
*/
public function registerpost($f3)
{
$this->validateCsrf();

// Exit immediately if public registrations are disabled
if (!$f3->get("site.public_registration")) {
Expand Down Expand Up @@ -184,6 +186,7 @@ public function reset($f3)
$f3->reroute("/");
} else {
if ($f3->get("POST.email")) {
$this->validateCsrf();
$user = new \Model\User();
$user->load(array("email = ?", $f3->get("POST.email")));
if ($user->id && !$user->deleted_date) {
Expand Down Expand Up @@ -233,6 +236,8 @@ public function reset_complete($f3, $params)
}

if ($f3->get("POST.password1")) {
$this->validateCsrf();

// Validate new password
if ($f3->get("POST.password1") != $f3->get("POST.password2")) {
$f3->set("reset.error", "The given passwords don't match.");
Expand Down Expand Up @@ -263,6 +268,10 @@ public function reset_forced($f3)
$user = new \Model\User();
$user->loadCurrent();

if ($f3->get('POST')) {
$this->validateCsrf();
}

if ($f3->get("POST.password1") != $f3->get("POST.password2")) {
$f3->set("reset.error", "The given passwords don't match.");
} elseif (strlen($f3->get("POST.password1")) < 6) {
Expand All @@ -280,12 +289,13 @@ public function reset_forced($f3)
}

/**
* GET|POST /logout
* POST /logout
*
* @param \Base $f3
*/
public function logout($f3)
{
$this->validateCsrf();
$session = new \Model\Session();
$session->loadCurrent();
$session->delete();
Expand Down
26 changes: 23 additions & 3 deletions app/controller/issues.php
View file
Expand Up @@ -239,6 +239,7 @@ public function index($f3)
*/
public function bulk_update($f3)
{
$this->validateCsrf();
$post = $f3->get("POST");

$issue = new \Model\Issue();
Expand Down Expand Up @@ -526,7 +527,7 @@ protected function loadIssueMeta(\Model\Issue $issue)
}

/**
* GET /issues/close/@id
* POST /issues/close/@id
* Close an issue
*
* @param \Base $f3
Expand All @@ -535,6 +536,7 @@ protected function loadIssueMeta(\Model\Issue $issue)
*/
public function close($f3, $params)
{
$this->validateCsrf();
$issue = new \Model\Issue();
$issue->load($params["id"]);

Expand All @@ -554,7 +556,7 @@ public function close($f3, $params)
}

/**
* GET /issues/reopen/@id
* POST /issues/reopen/@id
* Reopen an issue
*
* @param \Base $f3
Expand All @@ -563,6 +565,7 @@ public function close($f3, $params)
*/
public function reopen($f3, $params)
{
$this->validateCsrf();
$issue = new \Model\Issue();
$issue->load($params["id"]);

Expand All @@ -588,7 +591,7 @@ public function reopen($f3, $params)
}

/**
* GET /issues/copy/@id
* POST /issues/copy/@id
* Copy an issue
*
* @param \Base $f3
Expand All @@ -597,6 +600,7 @@ public function reopen($f3, $params)
*/
public function copy($f3, $params)
{
$this->validateCsrf();
$issue = new \Model\Issue();
$issue->load($params["id"]);

Expand Down Expand Up @@ -760,6 +764,7 @@ protected function _saveNew()
*/
public function save($f3)
{
$this->validateCsrf();
if ($f3->get("POST.id")) {
// Updating existing issue.
$issue = $this->_saveUpdate();
Expand Down Expand Up @@ -849,6 +854,8 @@ public function single($f3, $params)
*/
public function add_watcher($f3, $params)
{
$this->validateCsrf();

$issue = new \Model\Issue();
$issue->load(array("id=?", $params["id"]));
if (!$issue->id) {
Expand All @@ -875,6 +882,8 @@ public function add_watcher($f3, $params)
*/
public function delete_watcher($f3, $params)
{
$this->validateCsrf();

$issue = new \Model\Issue();
$issue->load(array("id=?", $params["id"]));
if (!$issue->id) {
Expand All @@ -896,6 +905,8 @@ public function delete_watcher($f3, $params)
*/
public function add_dependency($f3, $params)
{
$this->validateCsrf();

$issue = new \Model\Issue();
$issue->load(array("id=?", $params["id"]));
if (!$issue->id) {
Expand All @@ -921,6 +932,8 @@ public function add_dependency($f3, $params)
*/
public function delete_dependency($f3, $params)
{
$this->validateCsrf();

$issue = new \Model\Issue();
$issue->load(array("id=?", $params["id"]));
if (!$issue->id) {
Expand Down Expand Up @@ -1060,6 +1073,7 @@ public function single_dependencies($f3, $params)
*/
public function single_delete($f3, $params)
{
$this->validateCsrf();
$issue = new \Model\Issue();
$issue->load($params["id"]);
$user = $f3->get("user_obj");
Expand All @@ -1081,6 +1095,7 @@ public function single_delete($f3, $params)
*/
public function single_undelete($f3, $params)
{
$this->validateCsrf();
$issue = new \Model\Issue();
$issue->load($params["id"]);
$user = $f3->get("user_obj");
Expand All @@ -1101,6 +1116,7 @@ public function single_undelete($f3, $params)
*/
public function comment_save($f3)
{
$this->validateCsrf();
$post = $f3->get("POST");

$issue = new \Model\Issue();
Expand Down Expand Up @@ -1152,6 +1168,7 @@ public function comment_save($f3)
*/
public function comment_delete($f3)
{
$this->validateCsrf();
$this->_requireAdmin();
$comment = new \Model\Issue\Comment();
$comment->load($f3->get("POST.id"));
Expand All @@ -1168,6 +1185,7 @@ public function comment_delete($f3)
*/
public function file_delete($f3)
{
$this->validateCsrf();
$file = new \Model\Issue\File();
$file->load($f3->get("POST.id"));
$file->delete();
Expand All @@ -1183,6 +1201,7 @@ public function file_delete($f3)
*/
public function file_undelete($f3)
{
$this->validateCsrf();
$file = new \Model\Issue\File();
$file->load($f3->get("POST.id"));
$file->deleted_date = null;
Expand Down Expand Up @@ -1289,6 +1308,7 @@ public function search($f3)
*/
public function upload($f3)
{
$this->validateCsrf();
$user_id = $this->_userId;

$issue = new \Model\Issue();
Expand Down
3 changes: 3 additions & 0 deletions app/controller/taskboard.php
View file
Expand Up @@ -297,6 +297,7 @@ public function burndown($f3, $params)
*/
public function saveManHours($f3)
{
$this->validateCsrf();
$user = new \Model\User();
$user->load(["id = ?", $f3->get("POST.user_id")]);
if (!$user->id) {
Expand All @@ -316,6 +317,7 @@ public function saveManHours($f3)
*/
public function add($f3)
{
$this->validateCsrf();
$post = $f3->get("POST");
$post['sprint_id'] = $post['sprintId'];
$post['name'] = $post['title'];
Expand All @@ -333,6 +335,7 @@ public function add($f3)
*/
public function edit($f3)
{
$this->validateCsrf();
$post = $f3->get("POST");
$issue = new \Model\Issue();
$issue->load($post["taskId"]);
Expand Down

0 comments on commit 4032f44

Please sign in to comment.