Skip to content

Conversation

@wukunyu264
Copy link

@wukunyu264 wukunyu264 commented Aug 15, 2025

{PR title}

  • [ √] You've read the Contributor Guide and Code of Conduct.
  • [√ ] You've included unit or integration tests for your change, where applicable.
  • [√ ] You've included inline docs for your change, where applicable.
  • [ √] There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

{Detail}

Fixes #{63264}
The Bootstrap JS vendored by the sample contains three uses of:
/.*(?=#[^\s]+$)/
src/Security/samples/ClaimsTransformation/wwwroot/lib/bootstrap/dist/js/bootstrap.js(513,707,1267)
to keep the trailing #… part of a URL. This pattern can catastrophically backtrack under crafted inputs and cause high CPU.
Changes (two lines only)

line 514:

- href = href.replace(/.*(?=#[^\s]+$)/, '') // strip for ie7
+ href = href && href.indexOf('#') !== -1 ? href.slice(href.lastIndexOf('#')) : href // strip for ie7 (safe)

line 708:

- || (href = $trigger.attr('href')) && href.replace(/.*(?=#[^\s]+$)/, '') // strip for ie7
+ || ((href = $trigger.attr('href')) && (href.indexOf('#') !== -1 ? href.slice(href.lastIndexOf('#')) : href)) // strip for ie7 (safe)

line 1268:

- (href && href.replace(/.*(?=#[^\s]+$)/, '')) // strip for ie7
+ (href && href.replace(/^[^#]*(?=#\S+$)/, '')) // strip for ie7 - safe

Why

/.(?=#[^\s]+$)/ mixes a greedy . with an end-anchored look-ahead; inputs like very long runs of characters followed by a short terminator can trigger catastrophic backtracking (ReDoS).

The replacement using href.slice(href.lastIndexOf('#')) preserves exactly the same behavior (keep the trailing #… segment when present) with linear complexity and no new dependencies.

Tests

How to run:

git clone https://github.com/wukunyu264/aspnetcore.git
cd src/Security/samples/ClaimsTransformation/wwwroot/lib/bootstrap
node --test 

before:
image

image

after:
image

gist:https://gist.github.com/wukunyu264/05afd852d727cbc6d4ab533e363fd886
https://gist.github.com/wukunyu264/d6465afd5e9f30356a200e78110920cf
https://gist.github.com/wukunyu264/932fea8d20d9e4c5059fd119e4f463c0

@wukunyu264 wukunyu264 requested a review from halter73 as a code owner August 15, 2025 05:13
@github-actions github-actions bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Aug 15, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 15, 2025
@wukunyu264
Copy link
Author

@dotnet-policy-service agree

@wukunyu264
Copy link
Author

wukunyu264 commented Aug 15, 2025 via email

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Aug 22, 2025
@wukunyu264
Copy link
Author

Hi team - I have updated this PR: I have fixed another Redos vulnerability in the same file, resolved the conflict based on the latest main content, and included previous feedback. I also ran CI again to clear outdated status (/azp run).
The test passed locally; If you need anything else, please let me know. Thank you!

@ilonatommy ilonatommy added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Oct 17, 2025
@dotnet-policy-service
Copy link
Contributor

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants