Skip to content

Fix the unvalidated ?redirect= post-login navigation and harden safeRedirect #862

Closed

Description

@DevMuhdishaq

Description

The routed sign-in page navigates to an attacker-controlled ?redirect= query parameter without running it through safeRedirect, while the other two consumers of that parameter do validate. And safeRedirect itself only blocks values that do not start with / and those starting with //, so several well-known bypasses get through.

Where it lives

src/features/auth/components/SignInPage.tsx:27 — redirectTo = navigationState.from ?? searchParams.get('redirect') ?? '/' is passed to navigate() with no safeRedirect. Compare LoginPage.tsx:29,41 and GuestRoute.tsx:15, which do call it. The helper: src/features/auth/utils.ts:5-7 — it does not block /\evil.com, /%09/evil.com, or a javascript: value that survives normalization. Also SignInPage.tsx:18 casts fully attacker-influenceable router state with (location.state ?? {}) as NavigationState, and :26 renders notice from that state into the DOM. Separately src/features/auth/components/RequireAuth.tsx:23 writes state={{from: …}} that LoginPage never reads, so post-login redirects are broken app-wide.

Why it matters

An attacker can send a user to a phishing page that looks like the sign-in form, immediately after a real sign-in — the most effective phishing vector there is. A /\ value normalizes to // in some browsers, giving a protocol-relative navigation to an attacker host.

Suggested Direction

Extract safeRedirect to src/lib/ so it has one tested definition, and make it strict: reject anything not starting with a single /, reject / followed by \, and verify with new URL(value, location.origin).origin === location.origin. Apply it on every redirect target including navigationState.from. Unify RequireAuth and LoginPage on one redirect contract (state.from first, then ?redirect=, then a default) and validate at the single point of use. Never render router state directly without narrowing it.

Acceptance Criteria

  • ?redirect=//evil.com, ?redirect=/\evil.com and ?redirect=/%09/evil.com all resolve to a safe internal path
  • Every redirect target in the app is passed through one shared, tested safeRedirect
  • A post-login redirect preserves the originally requested destination
  • Router state is narrowed, not asserted, before use
  • Tests added/updated and passing (npm run test)
  • Docs updated if behavior changes (README.md / CONTRIBUTING.md as applicable)

Activity

  1. ravimahatonp commented on Oct 1, 2026

    @ravimahatonp

    Hello! I would like to work on this issue.

    I can tackle Fix the unvalidated ?redirect=post-login navigation and hardensafeRedirect``. Proposed implementation roadmap:

    • Analysis: Inspect current behavior and edge cases in Dfunder/stellarAid-web
    • Core Implementation: Trace and resolve the regression causing Fix the unvalidated ?redirect=post-login navigation and hardensafeRedirect`` with minimal code footprint.
    • Quality Assurance: Verify existing tests continue to pass without side effects.

    Looking forward to collaborating on this—please assign!

  2. pimaster900 commented on Oct 17, 2026

    @pimaster900

    @pimaster900 has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    i would like to work on this

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @pimaster900 to this issue.

  3. Maryermarh commented on Oct 17, 2026

    @Maryermarh
    Contributor

    @Maryermarh has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    Can I please work on this issue?

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @Maryermarh to this issue.

  4. drips-wave commented on Oct 17, 2026

    @drips-wave

    Congratulations, @Maryermarh! 🎉 Your application was accepted by the repo's maintainers, and the issue is due on October 22, 2026.

    🧑‍💻 @Maryermarh: Please resolve the issue such that the repo's maintainers have enough time to review your contribution before the due date. You'll earn Points for completing the issue on-time, which will make you eligible for a share of the Stellar Wave Program's reward pool.

    Warning

    When opening a PR, please link it to this issue to ensure it gets tracked accurately. Points are awarded when this issue is marked as completed by the maintainer.

    🤠 Repo maintainers: Please keep an eye on the contributor's progress and review their work before the due date. You can manage this issue, including adjusting its complexity and points, here.

    🌊 Happy Wave 🌊

  5. grantfox-oss commented on Oct 20, 2026

    @grantfox-oss

    🎉 This issue has been marked as completed on GrantFox!

    @Maryermarh's PR #1034 was approved and merged by @DevMuhdishaq.

    🏆 @Maryermarh: You earned 40 FoxPoints for this contribution! Your current tier: Builder (2,068 total points). Track your full progress on GrantFox.

    👏 Great work, @Maryermarh! Keep contributing to Dfunder.

  6. added a commit that references this issue on Oct 20, 2026
  7. drips-wave commented on Oct 20, 2026

    @drips-wave

    This issue has been completed by @Maryermarh as part of the Stellar Wave Program's 10th Wave 🥳

    😎 @Maryermarh: You earned 200 Points for completing this issue! Once the Wave's rewards are finalized, you'll be eligible for a percentage of the Wave's reward pool based on the percentage of total points you've earned. Learn more here. You can also Leave a review to share your experience working on this issue.

    🧑‍💻 Repo maintainers: How'd the contributor do? Leave a review to share your experience working with them.

  8. added a commit that references this issue on Oct 20, 2026
    a0d8876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions