feat(core): change the URL sanitization to only block javascript: URLs #49659

neuracr · 2023-03-31T11:53:30Z

In modern browsers, the 'javascript:' URL scheme is the only scheme that can execute JavaScript when passed in a navigation URL context (e.g. a.href value). Validate URL shemes to only contain characters allowed in the URL specification ([a-zA-Z-+.]), and that are not javascript (case insensitive). This is not a breaking change. The URL sanitization is loosen.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features). The documentation is already explaining the javascript: URLs are sanitized. This is still the case, and is the only scheme being sanitized.

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:
Security change.

What is the current behavior?

Issue Number: N/A

What is the new behavior?

The new sanitization algorithm validates the scheme has characters allowed by the URL specification. It is also blocklist based and blocks the javascript scheme (non case sensitive).

Does this PR introduce a breaking change?

Yes
No

Other information

bjarkler · 2023-03-31T12:57:26Z

@jelbourn LGTM for security.

bjarkler · 2023-03-31T12:58:01Z

packages/core/src/sanitization/url_sanitizer.ts

@@ -9,13 +9,14 @@
 import {XSS_SECURITY_URL} from '../error_details_base_url';

 /**
- * A pattern that recognizes a commonly useful subset of URLs that are safe.
+ * A pattern that recognizes a URLs that are safe wrt. XSS in URL navigation


nit: "recognizes a URLs" -> "recognizes URLs"

jessicajaniuk

reviewed-for: fw-core

jelbourn

LGTM

Reviewed-for: fw-security

In modern browsers, the 'javascript:' URL scheme is the only scheme that can execute JavaScript when passed in a navigation URL context (e.g. `a.href` value). Validate URL shemes to only contain characters allowed in the URL specification ([a-zA-Z-+.]), and that are not javascript (case insensitive). This is not a breaking change. The URL sanitization is loosen.

pkozlowski-opensource

Reviewed-for: fw-security

dylhunn · 2023-04-04T22:01:17Z

This PR was merged into the repository by commit b35fa73.

#49659) In modern browsers, the 'javascript:' URL scheme is the only scheme that can execute JavaScript when passed in a navigation URL context (e.g. `a.href` value). Validate URL shemes to only contain characters allowed in the URL specification ([a-zA-Z-+.]), and that are not javascript (case insensitive). This is not a breaking change. The URL sanitization is loosen. PR Close #49659

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.2.5/15.2.6) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.2.5/15.2.6) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.2.5/15.2.6) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.2.5/15.2.6) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.2.5/15.2.6) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.2.5/15.2.6) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.2.5/15.2.6) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`15.2.5` -> `15.2.6`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.2.5/15.2.6) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v15.2.6`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1526-2023-04-05) [Compare Source](angular/angular@15.2.5...15.2.6) ##### core | Commit | Type | Description | | -- | -- | -- | | [d9efa1b0d7](angular/angular@d9efa1b) | feat | change the URL sanitization to only block javascript: URLs ([#49659](angular/angular#49659)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [cad7274ef9](angular/angular@cad7274) | fix | create correct URL relative to path with empty child ([#49691](angular/angular#49691)) | | [9b61379096](angular/angular@9b61379) | fix | Ensure initial navigation clears current navigation when blocking ([#49572](angular/angular#49572)) | #### Special Thanks Andrew Scott, Guillaume Weghsteen, John Manners, Johnny Gérard, Matthieu Riegler, Robin Richtsfeld, Sandra Limacher, Sarthak Thakkar, Vinit Neogi and vikram menon  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1851 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2023-05-05T00:08:18Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

pullapprove bot requested review from alxhub, jelbourn, jessicajaniuk, josephperrott and pkozlowski-opensource March 31, 2023 11:53

angular-robot bot added the detected: feature PR contains a feature commit label Mar 31, 2023

bjarkler reviewed Mar 31, 2023

View reviewed changes

jessicajaniuk approved these changes Mar 31, 2023

View reviewed changes

jelbourn added security Issues that generally impact framework or application security area: security Issues related to built-in security features, such as HTML sanitation hotlist: google and removed detected: feature PR contains a feature commit labels Mar 31, 2023

ngbot bot modified the milestone: Backlog Mar 31, 2023

jelbourn approved these changes Mar 31, 2023

View reviewed changes

neuracr force-pushed the javascript-url-sanitizer branch from 63551df to bb41366 Compare April 3, 2023 13:41

angular-robot bot added the detected: feature PR contains a feature commit label Apr 3, 2023

pkozlowski-opensource removed request for pkozlowski-opensource, alxhub and josephperrott April 4, 2023 09:07

pullapprove bot requested review from alxhub, josephperrott and pkozlowski-opensource April 4, 2023 09:07

pkozlowski-opensource approved these changes Apr 4, 2023

View reviewed changes

pkozlowski-opensource removed request for alxhub and josephperrott April 4, 2023 09:09

pkozlowski-opensource added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Apr 4, 2023

dylhunn closed this in b35fa73 Apr 4, 2023

rubiesonthesky mentioned this pull request Apr 28, 2023

Add an ability to change SAFE_URL_PATTERN for app/service/component #49075

Open

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): change the URL sanitization to only block javascript: URLs #49659

feat(core): change the URL sanitization to only block javascript: URLs #49659

neuracr commented Mar 31, 2023

bjarkler commented Mar 31, 2023

bjarkler Mar 31, 2023

neuracr Apr 3, 2023

jessicajaniuk left a comment

jelbourn left a comment

pkozlowski-opensource left a comment

dylhunn commented Apr 4, 2023

angular-automatic-lock-bot bot commented May 5, 2023

feat(core): change the URL sanitization to only block javascript: URLs #49659

feat(core): change the URL sanitization to only block javascript: URLs #49659

Conversation

neuracr commented Mar 31, 2023

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

bjarkler commented Mar 31, 2023

bjarkler Mar 31, 2023

Choose a reason for hiding this comment

neuracr Apr 3, 2023

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

dylhunn commented Apr 4, 2023

angular-automatic-lock-bot bot commented May 5, 2023