Skip to content

[Snyk] Security upgrade react-router-dom from 5.3.3 to 6.30.2#3796

Open
simensma-fresh wants to merge 1 commit into
developfrom
snyk-fix-cc06cda7376925a59ee42a0c5c19cb58
Open

[Snyk] Security upgrade react-router-dom from 5.3.3 to 6.30.2#3796
simensma-fresh wants to merge 1 commit into
developfrom
snyk-fix-cc06cda7376925a59ee42a0c5c19cb58

Conversation

@simensma-fresh

Copy link
Copy Markdown
Collaborator

snyk-top-banner

Snyk has created this PR to fix 1 vulnerabilities in the yarn dependencies of this project.

Snyk changed the following file(s):

  • services/core-web/package.json

Note for zero-installs users

If you are using the Yarn feature zero-installs that was introduced in Yarn V2, note that this PR does not update the .yarn/cache/ directory meaning this code cannot be pulled and immediately developed on as one would expect for a zero-install project - you will need to run yarn to update the contents of the ./yarn/cache directory.
If you are not using zero-install you can ignore this as your flow should likely be unchanged.

⚠️ Warning
Failed to update the yarn.lock, please update manually before merging.

Vulnerabilities that will be fixed with an upgrade:

Issue Score
high severity Open Redirect
SNYK-JS-REACTROUTER-14908286
  641  

Important

  • Check the changes in this PR to ensure they won't cause issues with your project.
  • Max score is 1000. Note that the real score may have changed since the PR was raised.
  • This PR was automatically created by Snyk using the credentials of a real user.

Note: You are seeing this because you or someone else with access to this repository has authorized Snyk to open fix PRs.

For more information:
🧐 View latest project report
📜 Customise PR templates
🛠 Adjust project settings
📚 Read about Snyk's upgrade logic


Learn how to fix vulnerabilities with free interactive lessons:

🦉 Open Redirect

@alazar-aot

Copy link
Copy Markdown
Collaborator

Based on my Analysis, this PR introduces significant breaking changes that would require developer effort to resolve.

Key Points:

  • Going from v5 → v6 of the react-router-dom is a major change, as the codebase currently uses v5 API's extensively
  • Snyk raised this PR due to a high-severity open-redirect vulnerability in the react-router-dom package
    • Note: This is only exploitable if untrusted content is passed into navigation paths in the application code
  • I've identified ~10 locations where externally-sourced data flows into navigation calls, so this CVE may currently affect our application.fr

Breaking Changes:

Breaking Change v5 API v6 API Scope
<Switch> removed <Switch> <Routes> Routes.tsx, DashboardRoutes, SidebarWrapper, HelpGuide
Route component={} removed <Route component={X}> <Route element={<X />}> Several core-web and minespace-web routes
Route render={} removed <Route render={fn}> <Route element={<X />}> DashboardRoutes, RedirectRoute wrappers
exact prop removed <Route exact path="..."> Routes are exact by default Several route definitions
<Redirect> removed <Redirect to="..."> <Navigate to="..."> The ViewPermitRedirect, InactiveContact and RedirectRoute wrappers, DashboardRoutes
useHistory() removed const history = useHistory() const navigate = useNavigate() ~58 files
withRouter HOC removed export default withRouter(X) Use hooks individually ~25 files
<Prompt> removed <Prompt when={...}> unstable_useBlocker (unstable API) ~5 files
<Navigate> default is now push <Redirect> defaults to replace (overwrites current history entry) <Redirect> defaults to push (adds a new history entry) All redirect messages
activeClassName on <NavLink> removed activeClassName="active" className={({ isActive => ...} All NavLink ussage
matchPath signature reversed matchPath(path, options) matchPath(options, path) Anywhere matchPath is called
Nested Routes need <Outlet> Implicit child rendering Must add <Outlet /> to parent All layout/parent routes

Areas of Impact:

  • Below is a list of the key files where I've observed data flow into navigation calls:
    • AuthenticationGuard.tsx - HIGHEST RISK
    • NOWSideMenu.tsx
    • ReportsHomePage.tsx
    • MineReportInfo.tsx
    • ComplianceReportManagement.tsx
    • ScrollSideMenu.js
    • ProjectSummary.tsx
    • NotificationDrawer.tsx
  • AuthenticationGuard.tsx poses the highest risk as it manually constructs a full absolute URL using the un-checked input, while all the other files use dynamicRoute()
    • dynamicRoute() is defined per-route and always string-interpolates inputs into a hardcoded relative path, meaning the risk of untrusted inputs passed through producing an external URL are greatly reduced
    • Because AuthenticationGuard.tsx extracts the guid from a user-controlled URL with no validation, this seems to be the location in the codebase where an open-redirect attack via crafted URL is the most realistic

Migration Scale:

The following was generated by Claude and has not been extensively verified - I provided it the breaking changes table from above and asked it to search for all usages of the v5 APIs. This is the results.

  • ~299 files import from react-router-dom
  • ~58 files use useHistory (must each be migrated to useNavigate)
  • ~25 files use withRouter HOC (removed in v6 - requires refactoring each component to use hooks directly)
  • ~ 5 files use <Prompt> (require custom useBlocker-based implementation)
  • 60+ route definitions use the exact prop (no-op to remove, but must be done)

Summary:

  • Given that there appear to be instances where the underlying CVE may be present in our codebase, I believe these changes are worth investigating further

@asinn134 , @simensma-fresh , @matbusby FYI

@alazar-aot alazar-aot added the 🛑 DO NOT MERGE This pull request will have potentially destructive or risky changes if merged. label May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛑 DO NOT MERGE This pull request will have potentially destructive or risky changes if merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants