fix(federation): replace strcmp token oracle with hash-based comparison in requestSharedSecret#41579
Open
DeepDiver1975 wants to merge 2 commits into
Open
fix(federation): replace strcmp token oracle with hash-based comparison in requestSharedSecret#41579DeepDiver1975 wants to merge 2 commits into
DeepDiver1975 wants to merge 2 commits into
Conversation
The requestSharedSecret endpoint (@publicpage, unauthenticated) used strcmp() to compare the caller-supplied token against the stored local token, returning 403 when localToken > submitted_token and 200 otherwise. This binary oracle allows an attacker to binary-search the stored token in ~96 requests and then use it to obtain the federation shared secret. Replace strcmp($a, $b) with strcmp(hash("sha256",$a), hash("sha256",$b)). The deterministic tiebreaking property is preserved while the response reveals nothing about the plaintext token value. Signed-off-by: Thomas Müller <thomas.mueller@owncloud.com> Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
requestSharedSecret(@PublicPage, unauthenticated) usedstrcmp($localToken, $submitted)to decide 403 vs 200, leaking token ordering to any callergetSharedSecretto steal the federation shared secretSecurity Impact
High — unauthenticated callers can recover federation shared secrets when they know a trusted server URL
Test plan
testRequestSharedSecretNoOracleLeakageuses a token pair where oldstrcmpreturns 403 but hash-based returns 200; fails without fixdataTestRequestSharedSecretupdated with hash-ordered token pairsmake test TEST_PHP_SUITE=apps/federation🤖 Generated with Claude Code