recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Make the current session key a component of the CSRF token

Summary: Fixes T5510. This purely reduces false positives from HackerOne: we currently rotate CSRF tokens, but do not bind them explicitly to specific sessions. Doing so has no real security benefit and may make some session rotation changes more difficult down the line, but researchers routinely report it. Just conform to expectations since the expected behavior isn't bad and this is less work for us than dealing with false positives.

Test Plan:
- With two browsers logged in under the same user, verified I was issued different CSRF tokens.
- Verified the token from one browser did not work in the other browser's session.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5510

Differential Revision: https://secure.phabricator.com/D10136

epriestley 42cf7f6f 95eeffff

+7
+3
src/applications/auth/engine/PhabricatorAuthSessionEngine.php
··· 165 165 // TTL back up to the full duration. The idea here is that sessions are 166 166 // good forever if used regularly, but get GC'd when they fall out of use. 167 167 168 + // NOTE: If we begin rotating session keys when extending sessions, the 169 + // CSRF code needs to be updated so CSRF tokens survive session rotation. 170 + 168 171 if (time() + (0.80 * $ttl) > $session->getSessionExpires()) { 169 172 $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 170 173 $conn_w = $session_table->establishConnection('w');
+4
src/applications/people/storage/PhabricatorUser.php
··· 339 339 $vec = $this->getAlternateCSRFString(); 340 340 } 341 341 342 + if ($this->hasSession()) { 343 + $vec = $vec.$this->getSession()->getSessionKey(); 344 + } 345 + 342 346 $time_block = floor($epoch / $frequency); 343 347 $vec = $vec.$key.$time_block; 344 348