Skip to content

[minor][bugfix]: Remove CBA URL-string fallback (MSRC-42fca33e)#1829

Open
agubuzomaximus wants to merge 1 commit into
devfrom
maagubuzo/msrc/origin_confusion
Open

[minor][bugfix]: Remove CBA URL-string fallback (MSRC-42fca33e)#1829
agubuzomaximus wants to merge 1 commit into
devfrom
maagubuzo/msrc/origin_confusion

Conversation

@agubuzomaximus

Copy link
Copy Markdown
Contributor

PR Checklist (must be completed before review)

  • All tests pass locally
  • PR size is <= 500 LOC per PR Size Check policy
  • PR is independently mergeable (no hidden dependencies)
  • Appropriate reviewers are assigned
  • PR reviewed by code owner (required if Copilot-generated)
  • SME or Senior IC assigned where required

PR Title Format

Required Format: [Keyword1] [Keyword2]: Description

  • Keyword1: major, minor, or patch (case-insensitive)
  • Keyword2: feature, bugfix, engg, or tests (case-insensitive)

Examples:

  • [MAJOR] [Feature]: new API
  • [minor] [bugfix]: fix crash
  • [PATCH][tests]:add coverage

Proposed changes

Describe what this PR is trying to do.

Type of change

  • Feature work
  • Bug fix
  • Documentation
  • Engineering change
  • Test
  • Logging/Telemetry

Risk

  • High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
  • Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
  • Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

Copilot AI review requested due to automatic review settings May 15, 2026 23:45
@agubuzomaximus agubuzomaximus requested a review from a team as a code owner May 15, 2026 23:45
@agubuzomaximus agubuzomaximus changed the title [patch][bugfix]: Remove CBA URL-string fallback (MSRC-42fca33e) [minor][bugfix]: Remove CBA URL-string fallback (MSRC-42fca33e) May 15, 2026
@@ -46,16 +46,12 @@ + (BOOL)handleChallenge:(NSURLAuthenticationChallenge *)challenge
SecIdentityRef identity = NULL;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not update changelog.txt.

Please consider if this change would be noticeable to a partner or user and either update changelog.txt or resolve this conversation.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a security vulnerability (MSRC-42fca33e) by removing the URL-string fallback in MSIDCertAuthHandler that previously called SecIdentityCopyPreferred with webview.URL.absoluteString when the host-based lookup failed. That fallback enabled an origin-confusion attack vector where a webview URL host could be different from the protection space host. The PR also adds a unit test file with five tests covering the new behavior.

Changes:

  • Remove the URL-string fallback call to SecIdentityCopyPreferred in MSIDCertAuthHandler.m, so only the host-based lookup is used.
  • Add inline comments explaining the rationale for the removal.
  • Add MSIDCertAuthHandlerTests.m with five tests covering preferred identity lookup, fallback to user prompt, and certauth subdomain handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
IdentityCore/src/webview/embeddedWebview/challangeHandlers/mac/MSIDCertAuthHandler.m Removes URL-based SecIdentityCopyPreferred fallback; adds explanatory comment.
IdentityCore/tests/mac/MSIDCertAuthHandlerTests.m New test file validating the host-only lookup behavior, prompt fallback, and certauth subdomain flow.

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.

2 participants