Skip to content

fix: prevent IMS auth fragments from leaking into API org path - #908

Closed
kptdobe wants to merge 1 commit into
mainfrom
fix/ims-fragment-leaking-into-org-path
Closed

kptdobe wants to merge 1 commit into
mainfrom
fix/ims-fragment-leaking-into-org-path

Conversation

@kptdobe

@kptdobe kptdobe commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Problem

Two bugs in the client allow IMS OAuth redirect fragments to leak into admin API calls as an org name, producing requests like GET /config/ccess_token=<jwt>&state=.../ and GET /config/ld_hash=/. These caused 500 errors in da-admin (KV 414 key-too-long) and high-volume spurious 404s. See the worker-side fix in adobe/da-admin#270.


Bug 1 — blocks/edit/prose/index.js:263: unsafe hash.slice(2)

When a collab WebSocket connection closes and the document returns 404, the code navigates to the parent folder:

const split = window.location.hash.slice(2).split('/');
split.pop();
window.location.replace(`/#/${split.join('/')}`);

.slice(2) is meant to strip the #/ prefix. But when IMS has set the hash to #access_token=<jwt>&state=... (redirect still in progress), there is no / after #. .slice(2) removes #a instead, producing ccess_token=<jwt>&state=.... The code then navigates to /#/ccess_token=<jwt>&state=..., writing the mangled token into the URL as a path segment.

getPathDetails() picks up /ccess_token=... as the hash path and extracts ccess_token=<jwt>... as the org name. The resulting config API call sends a ~1986-byte key to Cloudflare KV (512-byte limit) → 500.

Fix: guard on hash.startsWith('#/') before slicing.


Bug 2 — blocks/shared/pathDetails.js:127: guard is dead code + ld_hash missing

const fullpath = hash.replace('#', '');  // always '/...' — hashPath always starts with '/'

// This can NEVER trigger: fullpath starts with '/', not 'old_hash' or 'access_token'
if (!fullpath || fullpath.startsWith('old_hash') || fullpath.startsWith('access_token')) return null;

fullpath is derived from hashPath, which is found by parts.find(part => part.startsWith('/')) — so it always starts with /. The prefix checks against 'old_hash' and 'access_token' (no leading /) can therefore never match. The guard is dead code.

Additionally, ld_hash — the IMS PKCE redirect marker — is not covered at all. When IMS sets the hash to #/ld_hash=, hashPath = '/ld_hash=' passes the startsWith('/') check, the dead guard ignores it, and org = 'ld_hash=' flows into the config API call → high-volume 404s (/config/ld_hash=/, ~500 requests/12h in production).

Fix: strip the leading / before the prefix checks, and add ld_hash.


Changes

File Change
blocks/edit/prose/index.js Add startsWith('#/') guard before hash.slice(2) in connection-close handler
blocks/shared/pathDetails.js Fix dead guard (strip leading / before prefix check), add ld_hash

Related

Worker-side defensive fix: adobe/da-admin#270 (catches any remaining cases where an oversized key reaches the KV call, returning 403 instead of 500).

🤖 Generated with Claude Code

Two bugs allow IMS OAuth redirect fragments to pass through as an org
name in admin API calls (e.g. /config/ccess_token=.../ or /config/ld_hash=/).

1. prose/index.js: hash.slice(2) strips '#a' instead of '#/' when the
   hash is '#access_token=...', writing 'ccess_token=...' into the URL.
   Guard with startsWith('#/') before slicing.

2. pathDetails.js: the IMS fragment guard was dead code — fullpath always
   starts with '/' so startsWith('old_hash') could never match. Also,
   'ld_hash' (used by the IMS PKCE flow) was not covered at all.
   Strip the leading '/' before the prefix checks and add 'ld_hash'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aem-code-sync

aem-code-sync Bot commented Apr 30, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@kptdobe kptdobe closed this Apr 30, 2026
@kptdobe
kptdobe deleted the fix/ims-fragment-leaking-into-org-path branch April 30, 2026 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant