fix(select): block xss on md-select-label #10023
Conversation
jelbourn
added
the
in progress
googlebot
added
the
cla: yes
jelbourn
force-pushed
the
md-select-xss
branch
4 times, most recently
from
a5a860e
to
1738662
Compare
| if (attr.mdSelectedHtml) { | ||
| if (!$injector.has('$sanitize')) { | ||
| throw Error('The ngSanitize module must be loaded in order to use ' + | ||
| 'md-treat-selected-text-as-html.'); |
There was a problem hiding this comment.
If there's no $sanitize, the user will get an $sce:unsafe error from getTrustedHtml, which points to the sanitizer already. It can make sense to not have it (codesize), and rely on $sce.trustAsHtml only instead, and you'll fail hard for them at this point. So from a purely security POV I wouldn't throw here or look for $sanitize, and just let $sce.getTrustedHtml do its thing: you already have the same pattern everywhere ng-bind-html is used.
|
Besides from the small comment, looks good :) thanks for the super-quick patch ! |
jelbourn
force-pushed
the
md-select-xss
branch
from
1738662
to
c611970
Compare
jelbourn
force-pushed
the
md-select-xss
branch
from
c611970
to
0b7da54
Compare
jelbourn
changed the title
jelbourn
added
needs: review
|
@ThomasBurleson @ErinCoughlan can you review? |
| // Using getTrustedHtml will run the content through $sanitize if it is not already | ||
| // explicitly trusted. If the ngSanitize module is not loaded, this will | ||
| // *correctly* throw an sce error. | ||
| target.html($sce.getTrustedHtml(text)); |
ThomasBurleson
added
P0: critical
|
@jelbourn - lgtm. |
|
This was a breaking change for users upgrading from 1.1.1 to 1.1.4/1.1.5 as mentioned in #10912. I'm going to update the changelog to add this to the breaking changes for 1.1.2. |
No description provided.