Skip to content

fix(timeslider): only follow a same-origin pad referrer in "Return to pad" - #8397

Merged
JohnMcLear merged 2 commits into
developfrom
fix/timeslider-return-open-redirect
Oct 28, 2026
Merged

JohnMcLear merged 2 commits into
developfrom
fix/timeslider-return-open-redirect

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

The timeslider's "Return to pad" button navigated to document.referrer whenever the character before its last / was p. A link from https://attacker/p/login (with referrerpolicy="unsafe-url") to /p/<pad>/timeslider?embed=1 therefore sent the user back to the attacker's site when they clicked the button. The button is hidden when the timeslider runs inside an iframe, so only direct top-level visits were affected.

The referrer is now parsed and only followed when it is a pad page on the same origin; otherwise the button returns to the pad. The logic lives in static/js/timesliderReturnUrl.ts so it can be unit tested.

Fixes GHSA-35rj-7vrx-29wq. Reported by @sun1024.

This PR also bumps develop to 3.3.11 (it was still at 3.3.10 after the 3.3.11 release) and adds the 3.3.12 changelog entry, which also credits @temantih for #8389.

Tests: new vitest spec (timesliderReturnUrl.test.ts: cross-origin, same-origin, empty and proxy-prefix cases); full vitest suite passes; tsc clean. Checked in Chromium against a local server: from a cross-origin /p/ page with unsafe-url, the button now returns to the pad instead of the attacker page.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V4cYEvi9Y4BTPntnkV9tzh

… pad"

The button navigated to document.referrer whenever the character before
its last "/" was "p", so a cross-origin link from https://attacker/p/login
to /p/<pad>/timeslider?embed=1 redirected the user off-site when they
clicked it (GHSA-35rj-7vrx-29wq, reported by @sun1024). Parse the
referrer and only follow it when it is a pad page on the same origin;
otherwise return to the pad.

Also bump develop to 3.3.11 (it was left at 3.3.10 after the 3.3.11
release) and add the 3.3.12 changelog entry, crediting @temantih for the
rev-export chat fix (#8389).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V4cYEvi9Y4BTPntnkV9tzh
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Prevent cross-origin redirects from timeslider’s Return to pad button

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Return to same-origin pad referrers only, preventing the timeslider button from sending users
 off-site.
• Add URL-resolution tests covering hostile referrers, fallback behavior, and reverse-proxy
 prefixes.
• Document the security release and align package versions with 3.3.11.
Diagram

graph TD
  A["Return button"] --> B["URL resolver"] --> C{"Same-origin pad?"} -->|Yes| D["Referrer URL"] --> F["Navigate"]
  C -->|No| E["Current pad URL"] --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Always return to the current pad
  • ➕ Eliminates referrer-dependent navigation and simplifies the security boundary.
  • ➖ Loses the existing behavior of returning to a qualifying pad referrer, including its URL parameters.

Recommendation: Keep the PR’s same-origin validation and pad fallback: it closes the cross-origin redirect while preserving the intended return behavior. Always using the current pad is simpler but changes that behavior.

Files changed (8) +76 / -12

Bug fix (2) +27 / -8
pad_editbar.tsUse validated return destination in the timeslider command +2/-8

Use validated return destination in the timeslider command

• Replaces the character-based referrer check with a call to the URL resolver before setting the browser location.

src/static/js/pad_editbar.ts

timesliderReturnUrl.tsResolve safe timeslider return URLs +25/-0

Resolve safe timeslider return URLs

• Adds a testable resolver that follows referrers only when their origin matches and their path contains '/p/'. Otherwise, it derives the current pad URL, retaining a proxy prefix and removing query and fragment components.

src/static/js/timesliderReturnUrl.ts

Tests (1) +36 / -0
timesliderReturnUrl.test.tsCover timeslider return URL security and fallback cases +36/-0

Cover timeslider return URL security and fallback cases

• Tests same-origin pad referrers, several cross-origin variants, empty or invalid referrers, non-pad paths, and reverse-proxy prefixes.

src/tests/backend-new/specs/timesliderReturnUrl.test.ts

Documentation (1) +9 / -0
CHANGELOG.mdDocument the 3.3.12 security release +9/-0

Document the 3.3.12 security release

• Adds an entry for the timeslider open-redirect fix and credits its reporter. Also documents the revision-bound export chat fix and credits its reporter.

CHANGELOG.md

Other (4) +4 / -4
package.jsonAlign admin package version with 3.3.11 +1/-1

Align admin package version with 3.3.11

• Changes the admin package version from 3.3.10 to 3.3.11.

admin/package.json

package.jsonAlign bin package version with 3.3.11 +1/-1

Align bin package version with 3.3.11

• Changes the bin package version from 3.3.10 to 3.3.11.

bin/package.json

package.jsonAlign root package version with 3.3.11 +1/-1

Align root package version with 3.3.11

• Changes the root package version from 3.3.10 to 3.3.11.

package.json

package.jsonAlign source package version with 3.3.11 +1/-1

Align source package version with 3.3.11

• Changes the source package version from 3.3.10 to 3.3.11.

src/package.json

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. The return button can reopen the timeslider ✓ Resolved
Description
getReturnToPadUrl accepts any same-origin pathname containing /p/, including
/p/testpad/timeslider?embed=1, rather than checking for the current pad page. If that URL is the
referrer, the return command follows it unchanged instead of navigating to /p/testpad; same-origin
export routes also pass the check.
Code

src/static/js/timesliderReturnUrl.ts[16]

+    if (ref.origin === here.origin && ref.pathname.includes('/p/')) return ref.href;
Evidence
The helper returns the entire referrer for any same-origin path containing /p/, and the command
uses that return value as its destination. The server defines both a timeslider route and export
routes beneath /p/:pad, while the pad page itself is /p/:pad; the new substring check does not
distinguish them.

src/static/js/timesliderReturnUrl.ts[12-24]
src/static/js/pad_editbar.ts[605-608]
src/node/hooks/express/specialpages.ts[400-401]
src/node/hooks/express/specialpages.ts[429-454]
src/node/hooks/express/importexport.ts[31-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The referrer check accepts same-origin timeslider and export URLs as pad pages, so Return to pad can navigate somewhere other than the current pad.
## Fix Focus Areas
- src/static/js/timesliderReturnUrl.ts[12-24]
- src/tests/backend-new/specs/timesliderReturnUrl.test.ts[8-35]
## Recommended Fix
Derive the current pad pathname from the timeslider URL and follow a referrer only when its origin and pathname match that pad page, accounting for any supported trailing slash. Preserve legitimate pad-page query strings, and add tests rejecting same-origin timeslider, export, and unrelated-pad referrers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/static/js/timesliderReturnUrl.ts Outdated
…s pad page

Addresses Qodo review: same-origin timeslider, export or other-pad URLs
no longer count as the pad to return to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V4cYEvi9Y4BTPntnkV9tzh
@JohnMcLear
JohnMcLear merged commit dbb1a03 into develop Oct 28, 2026
38 checks passed
@JohnMcLear
JohnMcLear deleted the fix/timeslider-return-open-redirect branch October 28, 2026 09:49
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