Fabric.js Affected by Stored XSS via SVG Export

@@ -2241,6 +2241,26 @@ describe('StaticCanvas', () => {
   // });
 });
 
+describe('malicious tests', () => {
+  it('from JSON to svg', async () => {
+    const canvas = new StaticCanvas();
+    const maliciousJSON = {
+      objects: [
+        {
+          type: 'rect',
+          id: '"><set onbegin="alert(1)"/>',
+          width: 100,
+          height: 100,
+          fill: 'red',
+        },
+      ],
+    };
+    await canvas.loadFromJSON(maliciousJSON);
+    const svg = canvas.toSVG();
+    expect(svg).not.toContain('onbegin="alert(1)"');
+  });
+});
+
 function makeRect(options = {}) {
   const defaultOptions = { width: 10, height: 10 };
   return new Rect({ ...defaultOptions, ...options });

@@ -44,6 +44,7 @@ import type { StaticCanvasOptions } from './StaticCanvasOptions';
 import { staticCanvasDefaults } from './StaticCanvasOptions';
 import { log, FabricError } from '../util/internals/console';
 import { getDevicePixelRatio } from '../env';
+import { escapeXml } from '../util/lang_string';
 
 /**
  * Having both options in TCanvasSizeOptions set to true transform the call in a calcOffset
@@ -950,7 +951,9 @@ export class StaticCanvas<
     this._setSVGPreamble(markup, options);
     this._setSVGHeader(markup, options);
     if (this.clipPath) {
-      markup.push(`<g clip-path="url(#${this.clipPath.clipPathId})" >\n`);
+      markup.push(
+        `<g clip-path="url(#${escapeXml(this.clipPath.clipPathId ?? '')})" >\n`,
+      );
     }
     this._setSVGBgOverlayColor(markup, 'background');
     this._setSVGBgOverlayImage(markup, 'backgroundImage', reviver);

@@ -158,6 +158,46 @@ describe('Gradient', () => {
       expect(gradient.toSVG(baseObj)).toEqualSVG(SVG_RADIAL);
     });
 
+    test('toSVG linear sanitizes coord injection', () => {
+      const gradient = new Gradient({
+        type: 'linear',
+        coords: {
+          x1: '0" /><script>alert(1)</script>' as unknown as number,
+          y1: '0" /><script>alert(1)</script>' as unknown as number,
+          x2: '0" /><script>alert(1)</script>' as unknown as number,
+          y2: '0" /><script>alert(1)</script>' as unknown as number,
+        },
+        colorStops: [
+          { offset: 0, color: 'rgba(255,0,0,0)' },
+          { offset: 1, color: 'green' },
+        ],
+      });
+      const baseObj = new FabricObject({ width: 100, height: 100 });
+      const svg = gradient.toSVG(baseObj);
+      expect(svg).not.toContain('<script>');
+    });
+
+    test('toSVG radial sanitizes coord injection', () => {
+      const gradient = new Gradient({
+        type: 'radial',
+        coords: {
+          x1: '0" /><script>alert(1)</script>' as unknown as number,
+          y1: '0" /><script>alert(1)</script>' as unknown as number,
+          x2: '0" /><script>alert(1)</script>' as unknown as number,
+          y2: '0" /><script>alert(1)</script>' as unknown as number,
+          r1: '0" /><script>alert(1)</script>' as unknown as number,
+          r2: '50" /><script>alert(1)</script>' as unknown as number,
+        },
+        colorStops: [
+          { offset: 0, color: 'red' },
+          { offset: 1, color: 'rgba(0,255,0,0)' },
+        ],
+      });
+      const baseObj = new FabricObject({ width: 100, height: 100 });
+      const svg = gradient.toSVG(baseObj);
+      expect(svg).not.toContain('<script>');
+    });
+
     test('toSVG radial with r1 > 0', () => {
       const gradient = createRadialGradientWithInternalRadius();
       const obj = new FabricObject({ width: 100, height: 100 });
@@ -825,4 +865,12 @@ describe('Gradient', () => {
     expect(gradient.colorStops[2].color).toEqual('rgba(0,0,0,1)');
     expect(gradient.colorStops[3].color).toEqual('rgba(0,0,0,1)');
   });
+
+  describe('Attrivbute injection', () => {
+    it('id injection', () => {
+      const gradient = new Gradient({ id: 'malicious"><script>' });
+      const svg = gradient.toSVG({} as any);
+      expect(svg).toContain('id="SVGID_malicious&quot;&gt;&lt;script&gt;');
+    });
+  });
 });

@@ -20,6 +20,7 @@ import type {
 } from './typedefs';
 import { classRegistry } from '../ClassRegistry';
 import { isPath } from '../util/typeAssertions';
+import { escapeXml } from '../util/lang_string';
 
 /**
  * Gradient class
@@ -209,47 +210,59 @@ export class Gradient<
     transform[5] -= offsetY;
 
     const commonAttributes = [
-      `id="SVGID_${this.id}"`,
+      `id="SVGID_${escapeXml(String(this.id))}"`,
       `gradientUnits="${gradientUnits}"`,
       `gradientTransform="${
         preTransform ? preTransform + ' ' : ''
       }${matrixToSVG(transform)}"`,
       '',
     ].join(' ');
 
+    const sanitizeCoord = (value: unknown) => parseFloat(String(value));
+
     if (this.type === 'linear') {
       const { x1, y1, x2, y2 } = this.coords;
+      const sx1 = sanitizeCoord(x1);
+      const sy1 = sanitizeCoord(y1);
+      const sx2 = sanitizeCoord(x2);
+      const sy2 = sanitizeCoord(y2);
       markup.push(
         '<linearGradient ',
         commonAttributes,
         ' x1="',
-        x1,
+        sx1,
         '" y1="',
-        y1,
+        sy1,
         '" x2="',
-        x2,
+        sx2,
         '" y2="',
-        y2,
+        sy2,
         '">\n',
       );
     } else if (this.type === 'radial') {
       const { x1, y1, x2, y2, r1, r2 } = this
         .coords as GradientCoords<'radial'>;
-      const needsSwap = r1 > r2;
+      const sx1 = sanitizeCoord(x1);
+      const sy1 = sanitizeCoord(y1);
+      const sx2 = sanitizeCoord(x2);
+      const sy2 = sanitizeCoord(y2);
+      const sr1 = sanitizeCoord(r1);
+      const sr2 = sanitizeCoord(r2);
+      const needsSwap = sr1 > sr2;
       // svg radial gradient has just 1 radius. the biggest.
       markup.push(
         '<radialGradient ',
         commonAttributes,
         ' cx="',
-        needsSwap ? x1 : x2,
+        needsSwap ? sx1 : sx2,
         '" cy="',
-        needsSwap ? y1 : y2,
+        needsSwap ? sy1 : sy2,
         '" r="',
-        needsSwap ? r1 : r2,
+        needsSwap ? sr1 : sr2,
         '" fx="',
-        needsSwap ? x2 : x1,
+        needsSwap ? sx2 : sx1,
         '" fy="',
-        needsSwap ? y2 : y1,
+        needsSwap ? sy2 : sy1,
         '">\n',
       );
       if (needsSwap) {
@@ -259,17 +272,17 @@ export class Gradient<
           colorStop.offset = 1 - colorStop.offset;
         });
       }
-      const minRadius = Math.min(r1, r2);
+      const minRadius = Math.min(sr1, sr2);
       if (minRadius > 0) {
         // i have to shift all colorStops and add new one in 0.
-        const maxRadius = Math.max(r1, r2),
+        const maxRadius = Math.max(sr1, sr2),
           percentageShift = minRadius / maxRadius;
         colorStops.forEach((colorStop) => {
           colorStop.offset += percentageShift * (1 - colorStop.offset);
         });
       }
     }
-
+    // todo make a malicious script tag injection test with color and also apply a fix with escapeXml
     colorStops.forEach(({ color, offset }) => {
       markup.push(
         `<stop offset="${offset * 100}%" style="stop-color:${color};"/>\n`,

@@ -187,4 +187,16 @@ describe('Pattern', () => {
     const obj = await Rect.fromObject(rectObj);
     expect(obj.fill instanceof Pattern).toBeTruthy();
   });
+
+  describe('attribute injection', () => {
+    it('escapes correctly the src', () => {
+      const pattern = new Pattern({
+        source: { src: '"><svg onload=alert(1)>', width: 10, height: 10 },
+      });
+      const svg = pattern.toSVG({ width: 100, height: 100 });
+      expect(svg).toContain(
+        'xlink:href="&quot;&gt;&lt;svg onload=alert(1)&gt;"',
+      );
+    });
+  });
 });

@@ -12,6 +12,7 @@ import type {
   SerializedPatternOptions,
 } from './types';
 import { log } from '../util/internals/console';
+import { escapeXml } from '../util/lang_string';
 
 /**
  * @see {@link http://fabric5.fabricjs.com/patterns demo}
@@ -177,12 +178,12 @@ export class Pattern {
           : ifNaN((patternSource as HTMLImageElement).height / height, 0);
 
     return [
-      `<pattern id="SVGID_${id}" x="${patternOffsetX}" y="${patternOffsetY}" width="${patternWidth}" height="${patternHeight}">`,
+      `<pattern id="SVGID_${escapeXml(id)}" x="${patternOffsetX}" y="${patternOffsetY}" width="${patternWidth}" height="${patternHeight}">`,
       `<image x="0" y="0" width="${
         (patternSource as HTMLImageElement).width
       }" height="${
         (patternSource as HTMLImageElement).height
-      }" xlink:href="${this.sourceToString()}"></image>`,
+      }" xlink:href="${escapeXml(this.sourceToString())}"></image>`,
       `</pattern>`,
       '',
     ].join('\n');

@@ -6,6 +6,7 @@ import { Point } from './Point';
 import type { FabricObject } from './shapes/Object/FabricObject';
 import type { TClassProperties } from './typedefs';
 import { uid } from './util/internals/uid';
+import { escapeXml } from './util/lang_string';
 import { pickBy } from './util/misc/pick';
 import { degreesToRadians } from './util/misc/radiansDegreesConversion';
 import { toFixed } from './util/misc/toFixed';
@@ -27,7 +28,6 @@ import { rotateVector } from './util/misc/vectors';
 
 (?:$|\s): This captures either the end of the line or a whitespace character. It ensures that the match ends either at the end of the string or with a whitespace character.
    */
-// eslint-disable-next-line max-len
 
 const shadowOffsetRegex = '(-?\\d+(?:\\.\\d*)?(?:px)?(?:\\s?|$))?';
 
@@ -105,7 +105,7 @@ export class Shadow {
    */
   declare nonScaling: boolean;
 
-  declare id: number;
+  declare id: number | string;
 
   static ownDefaults = shadowDefaultValues;
 
@@ -163,6 +163,7 @@ export class Shadow {
         degreesToRadians(-object.angle),
       ),
       BLUR_BOX = 20,
+      NUM_FRACTION_DIGITS = config.NUM_FRACTION_DIGITS,
       color = new Color(this.color);
     let fBoxX = 40,
       fBoxY = 40;
@@ -173,14 +174,14 @@ export class Shadow {
       fBoxX =
         toFixed(
           (Math.abs(offset.x) + this.blur) / object.width,
-          config.NUM_FRACTION_DIGITS,
+          NUM_FRACTION_DIGITS,
         ) *
           100 +
         BLUR_BOX;
       fBoxY =
         toFixed(
           (Math.abs(offset.y) + this.blur) / object.height,
-          config.NUM_FRACTION_DIGITS,
+          NUM_FRACTION_DIGITS,
         ) *
           100 +
         BLUR_BOX;
@@ -192,19 +193,19 @@ export class Shadow {
       offset.y *= -1;
     }
 
-    return `<filter id="SVGID_${this.id}" y="-${fBoxY}%" height="${
+    return `<filter id="SVGID_${escapeXml(this.id)}" y="-${fBoxY}%" height="${
       100 + 2 * fBoxY
     }%" x="-${fBoxX}%" width="${
       100 + 2 * fBoxX
     }%" >\n\t<feGaussianBlur in="SourceAlpha" stdDeviation="${toFixed(
       this.blur ? this.blur / 2 : 0,
-      config.NUM_FRACTION_DIGITS,
+      NUM_FRACTION_DIGITS,
     )}"></feGaussianBlur>\n\t<feOffset dx="${toFixed(
       offset.x,
-      config.NUM_FRACTION_DIGITS,
+      NUM_FRACTION_DIGITS,
     )}" dy="${toFixed(
       offset.y,
-      config.NUM_FRACTION_DIGITS,
+      NUM_FRACTION_DIGITS,
     )}" result="oBlur" ></feOffset>\n\t<feFlood flood-color="${color.toRgb()}" flood-opacity="${color.getAlpha()}"/>\n\t<feComposite in2="oBlur" operator="in" />\n\t<feMerge>\n\t\t<feMergeNode></feMergeNode>\n\t\t<feMergeNode in="SourceGraphic"></feMergeNode>\n\t</feMerge>\n</filter>\n`;
   }
 

@@ -10,6 +10,7 @@ import type { Abortable, TClassProperties, TOptions } from '../typedefs';
 import type { FabricObjectProps, SerializedObjectProps } from './Object/types';
 import type { CSSRules } from '../parser/typedefs';
 import { SCALE_X, SCALE_Y } from '../constants';
+import { escapeXml } from '../util/lang_string';
 
 interface UniqueCircleProps {
   /**
@@ -173,21 +174,21 @@ export class Circle<
    * of the instance
    */
   _toSVG(): string[] {
-    const angle = (this.endAngle - this.startAngle) % 360;
+    const { radius, startAngle, endAngle } = this;
+    const angle = (endAngle - startAngle) % 360;
 
     if (angle === 0) {
       return [
         '<circle ',
         'COMMON_PARTS',
         'cx="0" cy="0" ',
         'r="',
-        `${this.radius}`,
+        `${escapeXml(radius)}`,
         '" />\n',
       ];
     } else {
-      const { radius } = this;
-      const start = degreesToRadians(this.startAngle),
-        end = degreesToRadians(this.endAngle),
+      const start = degreesToRadians(startAngle),
+        end = degreesToRadians(endAngle),
         startX = cos(start) * radius,
         startY = sin(start) * radius,
         endX = cos(end) * radius,

@@ -7,6 +7,7 @@ import { FabricObject, cacheProperties } from './Object/FabricObject';
 import type { FabricObjectProps, SerializedObjectProps } from './Object/types';
 import type { ObjectEvents } from '../EventTypeDefs';
 import type { CSSRules } from '../parser/typedefs';
+import { escapeXml } from '../util/lang_string';
 
 export const ellipseDefaultValues: Partial<TClassProperties<Ellipse>> = {
   rx: 0,
@@ -127,7 +128,7 @@ export class Ellipse<
     return [
       '<ellipse ',
       'COMMON_PARTS',
-      `cx="0" cy="0" rx="${this.rx}" ry="${this.ry}" />\n`,
+      `cx="0" cy="0" rx="${escapeXml(this.rx)}" ry="${escapeXml(this.ry)}" />\n`,
     ];
   }
 

@@ -35,6 +35,7 @@ import {
 import type { SerializedLayoutManager } from '../LayoutManager/LayoutManager';
 import type { FitContentLayout } from '../LayoutManager';
 import type { DrawContext } from './Object/Object';
+import { escapeXml } from '../util/lang_string';
 
 /**
  * This class handles the specific case of creating a group using {@link Group#fromObject} and is not meant to be used in any other case.
@@ -649,7 +650,7 @@ export class Group
   getSvgStyles(): string {
     const opacity =
         typeof this.opacity !== 'undefined' && this.opacity !== 1
-          ? `opacity: ${this.opacity};`
+          ? `opacity: ${escapeXml(this.opacity)};`
           : '',
       visibility = this.visible ? '' : ' visibility: hidden;';
     return [opacity, this.getSvgFilter(), visibility].join('');

@@ -972,6 +972,18 @@ describe('FabricImage', () => {
       expect(img.scaleY).toBe(5);
     });
   });
+
+  describe('attribute injection', () => {
+    it('source injection', () => {
+      const element = newImg('javascript:alert(1)');
+      const img = new FabricImage(element, {
+        width: 100,
+        height: 100,
+      });
+      const svg = img.toSVG();
+      expect(svg).toContain('xlink:href="javascript:alert(1)"'); // Should be escaped
+    });
+  });
 });
 
 export function newImg(src = IMG_SRC): HTMLImageElement {

@@ -31,6 +31,7 @@ import type { CSSRules } from '../parser/typedefs';
 import type { Resize, ResizeSerializedProps } from '../filters/Resize';
 import type { TCachedFabricObject } from './Object/Object';
 import { log } from '../util/internals/console';
+import { escapeXml } from '../util/lang_string';
 
 // @todo Would be nice to have filtering code not imported directly.
 
@@ -389,9 +390,9 @@ export class FabricImage<
           '" y="' +
           y +
           '" width="' +
-          this.width +
+          escapeXml(this.width) +
           '" height="' +
-          this.height +
+          escapeXml(this.height) +
           '" />\n',
         '</clipPath>\n',
       );
@@ -403,7 +404,7 @@ export class FabricImage<
     imageMarkup.push(
       '\t<image ',
       'COMMON_PARTS',
-      `xlink:href="${this.getSvgSrc(true)}" x="${x - this.cropX}" y="${
+      `xlink:href="${escapeXml(this.getSrc(true))}" x="${x - this.cropX}" y="${
         y - this.cropY
         // we're essentially moving origin of transformation from top/left corner to the center of the shape
         // by wrapping it in container <g> element with actual transformation, then offsetting object to the top/left
@@ -419,9 +420,9 @@ export class FabricImage<
       const origFill = this.fill;
       this.fill = null;
       strokeSvg = [
-        `\t<rect x="${x}" y="${y}" width="${this.width}" height="${
-          this.height
-        }" style="${this.getSvgStyles()}" />\n`,
+        `\t<rect x="${x}" y="${y}" width="${escapeXml(this.width)}" height="${escapeXml(
+          this.height,
+        )}" style="${this.getSvgStyles()}" />\n`,
       ];
       this.fill = origFill;
     }
@@ -470,7 +471,10 @@ export class FabricImage<
    * @param {String} src Source string (URL)
    * @param {LoadImageOptions} [options] Options object
    */
-  setSrc(src: string, { crossOrigin, signal }: LoadImageOptions = {}) {
+  setSrc(
+    src: string,
+    { crossOrigin, signal }: LoadImageOptions = {},
+  ): Promise<void> {
     return loadImage(src, { crossOrigin, signal }).then((img) => {
       typeof crossOrigin !== 'undefined' && this.set({ crossOrigin });
       this.setElement(img);

@@ -199,18 +199,14 @@ export class Line<
    */
   calcLinePoints(): UniqueLineProps {
     const { x1: _x1, x2: _x2, y1: _y1, y2: _y2, width, height } = this;
-    const xMult = _x1 <= _x2 ? -1 : 1,
-      yMult = _y1 <= _y2 ? -1 : 1,
-      x1 = (xMult * width) / 2,
-      y1 = (yMult * height) / 2,
-      x2 = (xMult * -width) / 2,
-      y2 = (yMult * -height) / 2;
+    const xMult = _x1 <= _x2 ? -0.5 : 0.5,
+      yMult = _y1 <= _y2 ? -0.5 : 0.5;
 
     return {
-      x1,
-      x2,
-      y1,
-      y2,
+      x1: xMult * width,
+      x2: xMult * -width,
+      y1: yMult * height,
+      y2: yMult * -height,
     };
   }
 

@@ -5,6 +5,7 @@ import { FILL, NONE, STROKE } from '../../constants';
 import type { FabricObject } from './FabricObject';
 import { isFiller } from '../../util/typeAssertions';
 import { matrixToSVG } from '../../util/misc/svgExport';
+import { escapeXml } from '../../util/lang_string';
 
 export class FabricObjectSVGExportMixin {
   /**
@@ -67,15 +68,19 @@ export class FabricObjectSVGExportMixin {
       ';',
       filter,
       visibility,
-    ].join('');
+    ]
+      .map((v) => escapeXml(v))
+      .join('');
   }
 
   /**
    * Returns filter for svg shadow
    * @return {String}
    */
   getSvgFilter(this: FabricObjectSVGExportMixin & FabricObject) {
-    return this.shadow ? `filter: url(#SVGID_${this.shadow.id});` : '';
+    return this.shadow
+      ? `filter: url(#SVGID_${escapeXml(this.shadow.id)});`
+      : '';
   }
 
   /**
@@ -86,7 +91,7 @@ export class FabricObjectSVGExportMixin {
     this: FabricObjectSVGExportMixin & FabricObject & { id?: string },
   ) {
     return [
-      this.id ? `id="${this.id}" ` : '',
+      this.id ? `id="${escapeXml(String(this.id))}" ` : '',
       this.clipPath
         ? `clip-path="url(#${
             (this.clipPath as FabricObjectSVGExportMixin & FabricObject)
@@ -248,6 +253,8 @@ export class FabricObjectSVGExportMixin {
   }
 
   addPaintOrder(this: FabricObjectSVGExportMixin & FabricObject) {
-    return this.paintFirst !== FILL ? ` paint-order="${this.paintFirst}" ` : '';
+    return this.paintFirst !== FILL
+      ? ` paint-order="${escapeXml(this.paintFirst)}" `
+      : '';
   }
 }

@@ -0,0 +1,112 @@
+import { describe, expect, it } from 'vitest';
+import { Circle } from '../Circle';
+import { Ellipse } from '../Ellipse';
+import { Rect } from '../Rect';
+import { FabricText } from '../Text/Text';
+import { FabricImage } from '../Image';
+import { Shadow } from '../../Shadow';
+
+const MALICIOUS = 'x" /><script>alert(1)</script>';
+const MALICIOUS2 = `x" onclick="alert('svg animatetransform onbegin')"`;
+const ONCLICK_PAYLOAD = `onclick="alert('svg animatetransform onbegin')"`;
+
+describe.each([MALICIOUS, MALICIOUS2])(
+  'Object SVG export sanitization (%s)',
+  (payload) => {
+    it('sanitizes object id attributes', () => {
+      const rect = new Rect({
+        id: payload,
+        width: 10,
+        height: 10,
+      });
+
+      const svg = rect.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes object style attributes', () => {
+      const rect = new Rect({
+        width: 10,
+        height: 10,
+        fillRule: payload as unknown as 'nonzero',
+        strokeLineCap: payload as unknown as 'round',
+        strokeLineJoin: payload as unknown as 'round',
+        strokeDashArray: [payload as unknown as number],
+        paintFirst: payload as unknown as 'stroke',
+        shadow: new Shadow({
+          color: 'rgba(0, 0, 0, 0.5)',
+          blur: 0,
+          offsetX: 0,
+          offsetY: 0,
+        }),
+      });
+      rect.shadow.id = payload as unknown as number;
+
+      const svg = rect.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes circle radius output', () => {
+      const circle = new Circle({
+        radius: payload as unknown as number,
+      });
+
+      const svg = circle.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes ellipse radii output', () => {
+      const ellipse = new Ellipse({
+        rx: payload as unknown as number,
+        ry: payload as unknown as number,
+      });
+
+      const svg = ellipse.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes text content and font attributes', () => {
+      const text = new FabricText('<script>alert(1)</script>', {
+        fontFamily: `Times New Roman ${payload}`,
+      });
+
+      const svg = text.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes text style overrides', () => {
+      const text = new FabricText('x', {
+        styles: {
+          0: {
+            0: {
+              fill: `red ${payload}`,
+              fontFamily: `Times ${payload}`,
+              fontWeight: `bold ${payload}`,
+              fontStyle: `italic ${payload}`,
+              fontSize: payload as unknown as number,
+            },
+          },
+        },
+      });
+
+      const svg = text.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+
+    it('sanitizes image src output', () => {
+      const element = new Image(10, 10);
+      element.src = `data:image/svg+xml,<svg>${payload}</svg>`;
+      const image = new FabricImage(element, { width: 10, height: 10 });
+
+      const svg = image.toSVG();
+      expect(svg).not.toContain('<script>');
+      expect(svg).not.toContain(ONCLICK_PAYLOAD);
+    });
+  },
+);

@@ -213,11 +213,10 @@ export class Path<
    * of the instance
    */
   _toSVG() {
-    const path = joinPath(this.path, config.NUM_FRACTION_DIGITS);
     return [
       '<path ',
       'COMMON_PARTS',
-      `d="${path}" stroke-linecap="round" />\n`,
+      `d="${joinPath(this.path, config.NUM_FRACTION_DIGITS)}" stroke-linecap="round" />\n`,
     ];
   }
 

@@ -246,7 +246,7 @@ describe('Polygon', () => {
     expect(polygon.toSVG, 'toSVG should be a function').toBeTypeOf('function');
 
     const EXPECTED_SVG =
-      '<g transform="matrix(1 0 0 1 15 17)"  >\n<polygon style="stroke: rgb(0,0,255); stroke-width: 1; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(255,0,0); fill-rule: nonzero; opacity: 1;"  points="-5,-5 5,5 " />\n</g>\n';
+      '<g transform="matrix(1 0 0 1 15 17)"  >\n<polygon style="stroke: rgb(0,0,255); stroke-width: 1; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(255,0,0); fill-rule: nonzero; opacity: 1;"  points="-5,-5 5,5" />\n</g>\n';
 
     expect(polygon.toSVG(), 'SVG output should match expected').toBe(
       EXPECTED_SVG,

@@ -202,7 +202,7 @@ describe('Polyline', () => {
     const polyline = new Polygon(getPoints(), { fill: 'red', stroke: 'blue' });
     expect(polyline.toSVG).toBeTypeOf('function');
     const EXPECTED_SVG =
-      '<g transform="matrix(1 0 0 1 15 17)"  >\n<polygon style="stroke: rgb(0,0,255); stroke-width: 1; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(255,0,0); fill-rule: nonzero; opacity: 1;"  points="-5,-5 5,5 " />\n</g>\n';
+      '<g transform="matrix(1 0 0 1 15 17)"  >\n<polygon style="stroke: rgb(0,0,255); stroke-width: 1; stroke-dasharray: none; stroke-linecap: butt; stroke-dashoffset: 0; stroke-linejoin: miter; stroke-miterlimit: 4; fill: rgb(255,0,0); fill-rule: nonzero; opacity: 1;"  points="-5,-5 5,5" />\n</g>\n';
     expect(polyline.toSVG()).toEqual(EXPECTED_SVG);
   });
 

@@ -25,6 +25,7 @@ import {
   TOP,
 } from '../constants';
 import type { CSSRules } from '../parser/typedefs';
+import { escapeXml } from '../util/lang_string';
 
 export const polylineDefaultValues: Partial<TClassProperties<Polyline>> = {
   /**
@@ -323,27 +324,25 @@ export class Polyline<
    * of the instance
    */
   _toSVG() {
-    const points = [],
-      diffX = this.pathOffset.x,
+    const diffX = this.pathOffset.x,
       diffY = this.pathOffset.y,
       NUM_FRACTION_DIGITS = config.NUM_FRACTION_DIGITS;
 
-    for (let i = 0, len = this.points.length; i < len; i++) {
-      points.push(
-        toFixed(this.points[i].x - diffX, NUM_FRACTION_DIGITS),
-        ',',
-        toFixed(this.points[i].y - diffY, NUM_FRACTION_DIGITS),
-        ' ',
-      );
-    }
+    const points = this.points
+      .map(
+        ({ x, y }) =>
+          `${toFixed(x - diffX, NUM_FRACTION_DIGITS)},${toFixed(y - diffY, NUM_FRACTION_DIGITS)}`,
+      )
+      .join(' ');
+
     return [
       `<${
-        (this.constructor as typeof Polyline).type.toLowerCase() as
+        escapeXml((this.constructor as typeof Polyline).type).toLowerCase() as
           | 'polyline'
           | 'polygon'
       } `,
       'COMMON_PARTS',
-      `points="${points.join('')}" />\n`,
+      `points="${points}" />\n`,
     ];
   }
 

@@ -243,4 +243,26 @@ describe('Rect', () => {
     expect(rectObject.paintFirst).toBe('stroke');
     expect(rectSvg).toContain('paint-order="stroke"');
   });
+
+  describe('svg attribute injection', () => {
+    it('properties are properly escaped', () => {
+      const rect = new Rect({
+        id: 'asd"><script>alert(1)</script>',
+        width: 100,
+        height: 100,
+      });
+      const svg = rect.toSVG();
+      expect(svg).toContain(
+        `id="asd&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;"`,
+      );
+    });
+    it('polyglot test', () => {
+      const polyglotPayload =
+        'jaVasCript:/*-/*`/*\\`/*\'/*"/**/(/* */oNcliCk=alert() )';
+      const rect = new Rect({ id: polyglotPayload, width: 100, height: 100 });
+      const svg = rect.toSVG();
+      // Should escape all special characters
+      expect(svg).not.toContain(polyglotPayload);
+    });
+  });
 });

@@ -7,6 +7,7 @@ import { FabricObject, cacheProperties } from './Object/FabricObject';
 import type { FabricObjectProps, SerializedObjectProps } from './Object/types';
 import type { ObjectEvents } from '../EventTypeDefs';
 import type { CSSRules } from '../parser/typedefs';
+import { escapeXml } from '../util/lang_string';
 
 export const rectDefaultValues: Partial<TClassProperties<Rect>> = {
   rx: 0,
@@ -163,7 +164,7 @@ export class Rect<
       'COMMON_PARTS',
       `x="${-width / 2}" y="${
         -height / 2
-      }" rx="${rx}" ry="${ry}" width="${width}" height="${height}" />\n`,
+      }" rx="${escapeXml(rx)}" ry="${escapeXml(ry)}" width="${escapeXml(width)}" height="${escapeXml(height)}" />\n`,
     ];
   }
 

@@ -77,12 +77,12 @@ export class TextSVGExportMixin extends FabricObjectSVGExportMixin {
     return [
       textBgRects.join(''),
       '\t\t<text xml:space="preserve" ',
-      `font-family="${this.fontFamily.replace(dblQuoteRegex, "'")}" `,
-      `font-size="${this.fontSize}" `,
-      this.fontStyle ? `font-style="${this.fontStyle}" ` : '',
-      this.fontWeight ? `font-weight="${this.fontWeight}" ` : '',
+      `font-family="${escapeXml(this.fontFamily.replace(dblQuoteRegex, "'"))}" `,
+      `font-size="${escapeXml(this.fontSize)}" `,
+      this.fontStyle ? `font-style="${escapeXml(this.fontStyle)}" ` : '',
+      this.fontWeight ? `font-weight="${escapeXml(this.fontWeight)}" ` : '',
       textDecoration ? `text-decoration="${textDecoration}" ` : '',
-      this.direction === 'rtl' ? `direction="${this.direction}" ` : '',
+      this.direction === 'rtl' ? `direction="rtl" ` : '',
       'style="',
       this.getSvgStyles(noShadow),
       '"',
@@ -112,7 +112,7 @@ export class TextSVGExportMixin extends FabricObjectSVGExportMixin {
     // bounding-box background
     this.backgroundColor &&
       textBgRects.push(
-        ...createSVGInlineRect(
+        createSVGInlineRect(
           this.backgroundColor,
           -this.width / 2,
           -this.height / 2,
@@ -270,7 +270,7 @@ export class TextSVGExportMixin extends FabricObjectSVGExportMixin {
       if (currentColor !== lastColor) {
         lastColor &&
           textBgRects.push(
-            ...createSVGInlineRect(
+            createSVGInlineRect(
               lastColor,
               leftOffset + boxStart,
               textTopOffset,
@@ -287,7 +287,7 @@ export class TextSVGExportMixin extends FabricObjectSVGExportMixin {
     }
     currentColor &&
       textBgRects.push(
-        ...createSVGInlineRect(
+        createSVGInlineRect(
           lastColor,
           leftOffset + boxStart,
           textTopOffset,
@@ -339,17 +339,17 @@ export class TextSVGExportMixin extends FabricObjectSVGExportMixin {
     const thickness = textDecorationThickness || this.textDecorationThickness;
     return [
       stroke ? colorPropToSVG(STROKE, stroke) : '',
-      strokeWidth ? `stroke-width: ${strokeWidth}; ` : '',
+      strokeWidth ? `stroke-width: ${escapeXml(strokeWidth)}; ` : '',
       fontFamily
         ? `font-family: ${
             !fontFamily.includes("'") && !fontFamily.includes('"')
-              ? `'${fontFamily}'`
-              : fontFamily
+              ? `'${escapeXml(fontFamily)}'`
+              : escapeXml(fontFamily)
           }; `
         : '',
-      fontSize ? `font-size: ${fontSize}px; ` : '',
-      fontStyle ? `font-style: ${fontStyle}; ` : '',
-      fontWeight ? `font-weight: ${fontWeight}; ` : '',
+      fontSize ? `font-size: ${escapeXml(fontSize)}px; ` : '',
+      fontStyle ? `font-style: ${escapeXml(fontStyle)}; ` : '',
+      fontWeight ? `font-weight: ${escapeXml(fontWeight)}; ` : '',
       textDecoration
         ? `text-decoration: ${textDecoration}; text-decoration-thickness: ${toFixed((thickness * this.getObjectScaling().y) / 10, config.NUM_FRACTION_DIGITS)}%; `
         : '',

@@ -18,8 +18,9 @@ export const capitalize = (string: string, firstLetterOnly = false): string =>
  * @param {String} string String to escape
  * @return {String} Escaped version of a string
  */
-export const escapeXml = (string: string): string =>
-  string
+export const escapeXml = (stringOrNumber: string | number): string =>
+  stringOrNumber
+    .toString()
     .replace(/&/g, '&')
     .replace(/"/g, '"')
     .replace(/'/g, ''')

@@ -2,6 +2,7 @@ import { Color } from '../../color/Color';
 import { config } from '../../config';
 import { DEFAULT_SVG_FONT_SIZE, FILL, NONE } from '../../constants';
 import type { TBBox, SVGElementName, SupportedSVGUnit } from '../../typedefs';
+import { escapeXml } from '../lang_string';
 import { toFixed } from './toFixed';
 
 /**
@@ -133,7 +134,7 @@ export const colorPropToSVG = (
   if (!value) {
     colorValue = 'none';
   } else if (value.toLive) {
-    colorValue = `url(#SVGID_${value.id})`;
+    colorValue = `url(#SVGID_${escapeXml(value.id)})`;
   } else {
     const color = new Color(value),
       opacity = color.getAlpha();

@@ -29,5 +29,8 @@
     "e2e/test-report": true,
     "test/visual/assets": true
   },
-  "search.useIgnoreFiles": true
+  "search.useIgnoreFiles": true,
+  "[typescript]": {
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  }
 }