fix(common): sanitize rawSrc and rawSrcset values in NgOptimizedImage directive

AndrewKushnir · AndrewKushnir · commit a0fdb8c7e9ec · 2022-06-08T15:25:28.000-07:00
This commit applies a sanitization to values produced by a loader, before they are used for the `src` and `srcset` image element properties.
diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts
@@ -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';
 
@@ -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());
     }
@@ -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(', ');
   }
diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts
@@ -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();
