Skip to content

Commit

Permalink
fix(common): sanitize rawSrc and rawSrcset values in NgOptimizedI…
Browse files Browse the repository at this point in the history
…mage directive (#47082)

This commit applies a sanitization to values produced by a loader, before they are used for the `src` and `srcset` image element properties.

PR Close #47082
  • Loading branch information
AndrewKushnir authored and Pawel Kozlowski committed Aug 16, 2022
1 parent 3774d84 commit 1cf43de
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 3 deletions.
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, ElementRef, Inject, Injectable, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, OnInit, Renderer2, SimpleChanges, ɵformatRuntimeError as formatRuntimeError, ɵRuntimeError as RuntimeError} from '@angular/core';
import {Directive, ElementRef, Inject, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, OnInit, Renderer2, SimpleChanges, ɵ_sanitizeUrl as sanitizeUrl, ɵRuntimeError as RuntimeError} from '@angular/core';

import {RuntimeErrorCode} from '../../errors';

Expand Down Expand Up @@ -161,9 +161,15 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
}
this.setHostAttribute('loading', this.getLoadingBehavior());
this.setHostAttribute('fetchpriority', this.getFetchPriority());

// Use the `sanitizeUrl` function directly (vs using `DomSanitizer`),
// to make the code more tree-shakable (avoid referencing the entire class).
// The same function is used when regular `src` is used on an `<img>` element.
const src = sanitizeUrl(this.getRewrittenSrc());

// The `src` and `srcset` attributes should be set last since other attributes
// could affect the image's loading behavior.
this.setHostAttribute('src', this.getRewrittenSrc());
this.setHostAttribute('src', src);
if (this.rawSrcset) {
this.setHostAttribute('srcset', this.getRewrittenSrcset());
}
Expand Down Expand Up @@ -204,7 +210,8 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
const finalSrcs = this.rawSrcset.split(',').filter(src => src !== '').map(srcStr => {
srcStr = srcStr.trim();
const width = widthSrcSet ? parseFloat(srcStr) : parseFloat(srcStr) * this.width!;
return `${this.imageLoader({src: this.rawSrc, width})} ${srcStr}`;
const imgSrc = sanitizeUrl(this.imageLoader({src: this.rawSrc, width}));
return `${imgSrc} ${srcStr}`;
});
return finalSrcs.join(', ');
}
Expand Down
50 changes: 50 additions & 0 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -472,6 +472,56 @@ describe('Image directive', () => {
});
});

describe('sanitization', () => {
// Loader function that just returns provided `src`.
const noopLoader = (config: ImageLoaderConfig) => config.src;

it('should apply sanitization to the `rawSrc` with a static value', () => {
setupTestingModule({imageLoader: noopLoader});

const template =
'<img rawSrc="javascript:alert(`Unsafe code execution`)" width="50" height="50">';
const fixture = createTestComponent(template);
fixture.detectChanges();

const img = fixture.nativeElement.querySelector('img')!;
// The `src` looks like this: 'unsafe:javascript:alert(`Unsafe code execution`)'
expect(img.src.startsWith('unsafe:')).toBe(true);
});

it('should apply sanitization to the `rawSrc` when used as a binding', () => {
setupTestingModule({imageLoader: noopLoader});

const template =
'<img [rawSrc]="\'javascript:alert(`Unsafe code execution`)\'" width="50" height="50">';
const fixture = createTestComponent(template);
fixture.detectChanges();

const img = fixture.nativeElement.querySelector('img')!;
// The `src` looks like this: 'unsafe:javascript:alert(`Unsafe code execution`)'
expect(img.src.startsWith('unsafe:')).toBe(true);
});

it('should apply sanitization to the `rawSrcset` value', () => {
setupTestingModule({imageLoader: noopLoader});

const template =
`<img rawSrc="javascript:alert(\`Unsafe code execution\`)" rawSrcset="100w, 200w" width="100" height="50">`;
const fixture = createTestComponent(template);
fixture.detectChanges();

const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;

// The `src` looks like this: 'unsafe:javascript:alert(`Unsafe code execution`)'
expect(img.src.startsWith('unsafe:')).toBe(true);
expect(img.srcset)
.toBe(
'unsafe:javascript:alert(`Unsafe code execution`) 100w, ' +
'unsafe:javascript:alert(`Unsafe code execution`) 200w');
});
});

describe('fetch priority', () => {
it('should be "high" for priority images', () => {
setupTestingModule();
Expand Down

0 comments on commit 1cf43de

Please sign in to comment.