Page MenuHomePhabricator
Differential D14026

Use phutil_hashes_are_identical() when comparing hashes in Phabricator
ClosedPublic
Actions

Authored by epriestley on Sep 1 2015, 2:39 AM.
Tags
None
Referenced Files
F18421895: D14026.diff
Sat, Aug 30, 5:29 PM
F18179665: D14026.id33913.diff
Sat, Aug 16, 4:00 AM
F18112816: D14026.id.diff
Tue, Aug 12, 9:00 PM
F18110100: D14026.diff
Mon, Aug 11, 4:10 PM
F18085148: D14026.id.diff
Tue, Aug 5, 7:01 PM
F17937986: D14026.diff
Jul 31 2025, 1:45 AM
Unknown Object (File)
Jun 27 2025, 5:07 PM
Unknown Object (File)
Jun 15 2025, 5:09 AM
View All Files
Subscribers
• chenxiruanhai

Details

Reviewers
chad
Commits
Restricted Diffusion Commit
rP29948eaa5bd2: Use phutil_hashes_are_identical() when comparing hashes in Phabricator
Summary

See D14025. In all cases where we compare hashes, use strict, constant-time comparisons.

Test Plan

Logged in, logged out, added TOTP, ran Conduit, terminated sessions, submitted forms, changed password. Tweaked CSRF token, got rejected.

Diff Detail

Repository
rP Phabricator
Branch
hashcomp
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 7868
Build 8767: [Placeholder Plan] Wait for 30 Seconds
Build 8766: arc lint + arc unit

Event Timeline

epriestley updated this revision to Diff 33913.Sep 1 2015, 2:39 AM
epriestley retitled this revision from to Use phutil_hashes_are_identical() when comparing hashes in Phabricator.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
epriestley added inline comments.Sep 1 2015, 2:41 AM
src/applications/people/storage/PhabricatorUser.php
411–412

The extra changes here are just following through with this and removing support for the plain CSRF tokens. The BREACH tokens have been in the wild for a little over a year, now.

chad accepted this revision.Sep 1 2015, 2:46 AM
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 1 2015, 2:46 AM
• chenxiruanhai added a subscriber: • chenxiruanhai.Sep 1 2015, 8:58 AM

this is a test.

Closed by commit rP29948eaa5bd2: Use phutil_hashes_are_identical() when comparing hashes in Phabricator (authored by epriestley, committed by epriestley). · Explain WhySep 1 2015, 10:52 PM
This revision was automatically updated to reflect the committed changes.
• cyn0228.ch added a task: T5375: Tokenizers fail in Firefox on Android.Sep 7 2015, 10:42 PM
chad removed a task: T5375: Tokenizers fail in Firefox on Android.Sep 7 2015, 10:45 PM

Revision Contents
Changeset List

PathSize
resources/
celerity/
20 lines
src/
applications/
auth/
controller/
2 lines
5 lines
engine/
5 lines
factor/
2 lines
provider/
2 lines
conduit/
controller/
3 lines
method/
2 lines
metamta/
controller/
4 lines
people/
storage/
36 lines
settings/
panel/
5 lines
infrastructure/
util/
password/
2 lines

Diff 33913

View Options

resources/celerity/map.php

Loading...
View Options

src/applications/auth/controller/PhabricatorAuthController.php

Loading...
View Options

src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php

Loading...
View Options

src/applications/auth/engine/PhabricatorAuthSessionEngine.php

Loading...
View Options

src/applications/auth/factor/PhabricatorTOTPAuthFactor.php

Loading...
View Options

src/applications/auth/provider/PhabricatorAuthProvider.php

Loading...
View Options

src/applications/conduit/controller/PhabricatorConduitAPIController.php

Loading...
View Options

src/applications/conduit/method/ConduitConnectConduitAPIMethod.php

Loading...
View Options

src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php

Loading...
View Options

src/applications/people/storage/PhabricatorUser.php

Loading...
View Options

src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php

Loading...
View Options

src/infrastructure/util/password/PhabricatorPasswordHasher.php

Loading...
Phabricator · Built by Phacility