1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-28 08:20:57 +01:00
phorge-phorge/src/aphront/exception/AphrontCSRFException.php

22 lines
667 B
PHP
Raw Normal View History

Fix conservative CSRF token cycling limit Summary: We currently cycle CSRF tokens every hour and check for the last two valid ones. This means that a form could go stale in as little as an hour, and is certainly stale after two. When a stale form is submitted, you basically get a terrible heisen-state where some of your data might persist if you're lucky but more likely it all just vanishes. The .js file below outlines some more details. This is a pretty terrible UX and we don't need to be as conservative about CSRF validation as we're being. Remedy this problem by: - Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are valid for at least 6 hours, and for as long as 7). - Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected to the internet are valid indefinitely). - Showing the user an explicit message about what went wrong when CSRF validation fails so the experience is less bewildering. They should now only be able to submit with a bad CSRF token if: - They load a page, disconnect from the internet for 7 hours, reconnect, and submit the form within 55 minutes; or - They are actually the victim of a CSRF attack. We could eventually fix the first one by tracking reconnects, which might be "free" once the notification server gets built. It will probably never be an issue in practice. Test Plan: - Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15 seconds, got the CSRF exception. - Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15 seconds, got a clean form post. - Added debugging code the the csrf refresh to make sure it was doing sensible things (pulling different tokens, finding all the inputs). Reviewed By: aran Reviewers: tuomaspelkonen, jungejason, aran CC: aran, epriestley Differential Revision: 660
2011-07-13 23:05:18 +02:00
<?php
/*
* Copyright 2012 Facebook, Inc.
Fix conservative CSRF token cycling limit Summary: We currently cycle CSRF tokens every hour and check for the last two valid ones. This means that a form could go stale in as little as an hour, and is certainly stale after two. When a stale form is submitted, you basically get a terrible heisen-state where some of your data might persist if you're lucky but more likely it all just vanishes. The .js file below outlines some more details. This is a pretty terrible UX and we don't need to be as conservative about CSRF validation as we're being. Remedy this problem by: - Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are valid for at least 6 hours, and for as long as 7). - Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected to the internet are valid indefinitely). - Showing the user an explicit message about what went wrong when CSRF validation fails so the experience is less bewildering. They should now only be able to submit with a bad CSRF token if: - They load a page, disconnect from the internet for 7 hours, reconnect, and submit the form within 55 minutes; or - They are actually the victim of a CSRF attack. We could eventually fix the first one by tracking reconnects, which might be "free" once the notification server gets built. It will probably never be an issue in practice. Test Plan: - Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15 seconds, got the CSRF exception. - Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15 seconds, got a clean form post. - Added debugging code the the csrf refresh to make sure it was doing sensible things (pulling different tokens, finding all the inputs). Reviewed By: aran Reviewers: tuomaspelkonen, jungejason, aran CC: aran, epriestley Differential Revision: 660
2011-07-13 23:05:18 +02:00
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
final class AphrontCSRFException extends AphrontException {
Fix conservative CSRF token cycling limit Summary: We currently cycle CSRF tokens every hour and check for the last two valid ones. This means that a form could go stale in as little as an hour, and is certainly stale after two. When a stale form is submitted, you basically get a terrible heisen-state where some of your data might persist if you're lucky but more likely it all just vanishes. The .js file below outlines some more details. This is a pretty terrible UX and we don't need to be as conservative about CSRF validation as we're being. Remedy this problem by: - Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are valid for at least 6 hours, and for as long as 7). - Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected to the internet are valid indefinitely). - Showing the user an explicit message about what went wrong when CSRF validation fails so the experience is less bewildering. They should now only be able to submit with a bad CSRF token if: - They load a page, disconnect from the internet for 7 hours, reconnect, and submit the form within 55 minutes; or - They are actually the victim of a CSRF attack. We could eventually fix the first one by tracking reconnects, which might be "free" once the notification server gets built. It will probably never be an issue in practice. Test Plan: - Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15 seconds, got the CSRF exception. - Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15 seconds, got a clean form post. - Added debugging code the the csrf refresh to make sure it was doing sensible things (pulling different tokens, finding all the inputs). Reviewed By: aran Reviewers: tuomaspelkonen, jungejason, aran CC: aran, epriestley Differential Revision: 660
2011-07-13 23:05:18 +02:00
}