diff --git a/SECURITY.md b/SECURITY.md index 6aaee3b9..d19ea627 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -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. diff --git a/app/controller.php b/app/controller.php index 0c77aaa9..3a841e9c 100644 --- a/app/controller.php +++ b/app/controller.php @@ -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(); + } } diff --git a/app/controller/admin.php b/app/controller/admin.php index 612743f4..406ada7f 100644 --- a/app/controller/admin.php +++ b/app/controller/admin.php @@ -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 @@ -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")); @@ -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"); @@ -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) { @@ -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) { @@ -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"); } /** @@ -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(); @@ -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); } @@ -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(); @@ -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")) { diff --git a/app/controller/backlog.php b/app/controller/backlog.php index dbf2b56c..935af134 100644 --- a/app/controller/backlog.php +++ b/app/controller/backlog.php @@ -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(); @@ -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")) { diff --git a/app/controller/index.php b/app/controller/index.php index 0728e3a1..a515fbd8 100644 --- a/app/controller/index.php +++ b/app/controller/index.php @@ -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 @@ -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")) { @@ -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) { @@ -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."); @@ -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) { @@ -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(); diff --git a/app/controller/issues.php b/app/controller/issues.php index df8d00dc..1c1471c4 100644 --- a/app/controller/issues.php +++ b/app/controller/issues.php @@ -239,6 +239,7 @@ public function index($f3) */ public function bulk_update($f3) { + $this->validateCsrf(); $post = $f3->get("POST"); $issue = new \Model\Issue(); @@ -526,7 +527,7 @@ protected function loadIssueMeta(\Model\Issue $issue) } /** - * GET /issues/close/@id + * POST /issues/close/@id * Close an issue * * @param \Base $f3 @@ -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"]); @@ -554,7 +556,7 @@ public function close($f3, $params) } /** - * GET /issues/reopen/@id + * POST /issues/reopen/@id * Reopen an issue * * @param \Base $f3 @@ -563,6 +565,7 @@ public function close($f3, $params) */ public function reopen($f3, $params) { + $this->validateCsrf(); $issue = new \Model\Issue(); $issue->load($params["id"]); @@ -588,7 +591,7 @@ public function reopen($f3, $params) } /** - * GET /issues/copy/@id + * POST /issues/copy/@id * Copy an issue * * @param \Base $f3 @@ -597,6 +600,7 @@ public function reopen($f3, $params) */ public function copy($f3, $params) { + $this->validateCsrf(); $issue = new \Model\Issue(); $issue->load($params["id"]); @@ -760,6 +764,7 @@ protected function _saveNew() */ public function save($f3) { + $this->validateCsrf(); if ($f3->get("POST.id")) { // Updating existing issue. $issue = $this->_saveUpdate(); @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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"); @@ -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"); @@ -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(); @@ -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")); @@ -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(); @@ -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; @@ -1289,6 +1308,7 @@ public function search($f3) */ public function upload($f3) { + $this->validateCsrf(); $user_id = $this->_userId; $issue = new \Model\Issue(); diff --git a/app/controller/taskboard.php b/app/controller/taskboard.php index e089f18b..8d7772d0 100644 --- a/app/controller/taskboard.php +++ b/app/controller/taskboard.php @@ -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) { @@ -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']; @@ -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"]); diff --git a/app/controller/user.php b/app/controller/user.php index 8be269fe..cac72afc 100644 --- a/app/controller/user.php +++ b/app/controller/user.php @@ -82,6 +82,7 @@ public function dashboard($f3) */ public function dashboardPost($f3) { + $this->validateCsrf(); $helper = \Helper\Dashboard::instance(); $widgets = json_decode($f3->get("POST.widgets")); $allWidgets = $helper->allWidgets; @@ -145,6 +146,7 @@ public function account($f3) */ public function save($f3) { + $this->validateCsrf(); $f3 = \Base::instance(); $post = array_map("trim", $f3->get("POST")); @@ -234,6 +236,7 @@ public function save($f3) */ public function avatar($f3) { + $this->validateCsrf(); $f3 = \Base::instance(); $user = new \Model\User(); diff --git a/app/helper/security.php b/app/helper/security.php index 43ab67d2..51aa3805 100644 --- a/app/helper/security.php +++ b/app/helper/security.php @@ -113,6 +113,36 @@ public function updateDatabase($version) } } + /** + * Initialize a CSRF token + */ + public function initCsrfToken() + { + $f3 = \Base::instance(); + if (!($token = $f3->get('COOKIE.XSRF-TOKEN'))) { + $token = $this->salt_sha2(); + $f3->set('COOKIE.XSRF-TOKEN', $token); + } + $f3->set('csrf_token', $token); + } + + /** + * Validate a CSRF token, exiting if invalid + */ + public function validateCsrfToken() + { + $f3 = \Base::instance(); + $cookieToken = $f3->get('COOKIE.XSRF-TOKEN'); + $requestToken = $f3->get('POST.csrf-token'); + if (!$requestToken) { + $requestToken = $f3->get('HEADERS.X-Xsrf-Token'); + } + if (!$cookieToken || !$requestToken || !hash_equals($cookieToken, $requestToken)) { + $f3->error(400, 'Invalid CSRF token'); + exit; + } + } + /** * Check if two hashes are equal, safe against timing attacks * diff --git a/app/helper/template.php b/app/helper/template.php new file mode 100644 index 00000000..d6d12112 --- /dev/null +++ b/app/helper/template.php @@ -0,0 +1,11 @@ +esc($csrf_token) ?>" />'; + } +} diff --git a/app/routes.ini b/app/routes.ini index 54c24e8a..032723c5 100644 --- a/app/routes.ini +++ b/app/routes.ini @@ -8,8 +8,7 @@ GET|POST /reset/forced = Controller\Index->reset_forced GET|POST /reset/@token = Controller\Index->reset_complete POST /login = Controller\Index->loginpost POST /register = Controller\Index->registerpost -GET|POST /logout = Controller\Index->logout -GET|POST /ping = Controller\Index->ping +POST /logout = Controller\Index->logout GET /opensearch.xml = Controller\Index->opensearch ; Issues @@ -21,9 +20,9 @@ GET /issues/edit/@id = Controller\Issues->edit POST /issues/save = Controller\Issues->save POST /issues/bulk_update = Controller\Issues->bulk_update GET /issues/export = Controller\Issues->export -GET|POST /issues/@id = Controller\Issues->single -GET|POST /issues/delete/@id = Controller\Issues->single_delete -GET|POST /issues/undelete/@id = Controller\Issues->single_undelete +GET /issues/@id = Controller\Issues->single +POST /issues/delete/@id = Controller\Issues->single_delete +POST /issues/undelete/@id = Controller\Issues->single_undelete POST /issues/comment/save = Controller\Issues->comment_save POST /issues/comment/delete = Controller\Issues->comment_delete POST /issues/file/delete = Controller\Issues->file_delete @@ -40,9 +39,9 @@ GET /issues/project/@id = Controller\Issues\Project->overview GET /issues/project/@id/files = Controller\Issues\Project->files GET /search = Controller\Issues->search POST /issues/upload = Controller\Issues->upload -GET /issues/close/@id = Controller\Issues->close -GET /issues/reopen/@id = Controller\Issues->reopen -GET /issues/copy/@id = Controller\Issues->copy +POST /issues/close/@id = Controller\Issues->close +POST /issues/reopen/@id = Controller\Issues->reopen +POST /issues/copy/@id = Controller\Issues->copy GET /issues/parent_ajax = Controller\Issues->parent_ajax ; Tags @@ -74,19 +73,15 @@ GET /admin/users/deleted = Controller\Admin->deleted_users GET /admin/users/new = Controller\Admin->user_new POST /admin/users/save = Controller\Admin->user_save GET /admin/users/@id = Controller\Admin->user_edit -GET|POST /admin/users/@id/delete = Controller\Admin->user_delete -GET|POST /admin/users/@id/undelete = Controller\Admin->user_undelete +POST /admin/users/@id/delete = Controller\Admin->user_delete +POST /admin/users/@id/undelete = Controller\Admin->user_undelete POST /admin/groups/new = Controller\Admin->group_new -GET|POST /admin/groups/@id = Controller\Admin->group_edit -GET|POST /admin/groups/@id/delete = Controller\Admin->group_delete +GET /admin/groups/@id = Controller\Admin->group_edit +POST /admin/groups/@id/delete = Controller\Admin->group_delete POST /admin/groups/ajax = Controller\Admin->group_ajax GET|POST /admin/groups/@id/setmanager/@user_group_id = Controller\Admin->group_setmanager -GET|POST /admin/attributes/new = Controller\Admin->attribute_new -GET|POST /admin/attributes/@id = Controller\Admin->attribute_edit -GET|POST /admin/attributes/@id/delete = Controller\Admin->attribute_delete - GET|POST /admin/sprints/new = Controller\Admin->sprint_new GET|POST /admin/sprints/@id = Controller\Admin->sprint_edit diff --git a/app/view/admin/config.html b/app/view/admin/config.html index 7a1040ec..97521ac1 100644 --- a/app/view/admin/config.html +++ b/app/view/admin/config.html @@ -8,6 +8,7 @@
+
diff --git a/app/view/admin/groups.html b/app/view/admin/groups.html index e560ef64..91b3fd50 100644 --- a/app/view/admin/groups.html +++ b/app/view/admin/groups.html @@ -32,7 +32,14 @@ {{ @group.name | esc }} {{ @group.count }}  #{{ @group.task_color }} - + + + + + + @@ -52,6 +59,7 @@