Image checking security improvement (#18)

* Image checking security improvement
microsoft · Jan 17, 2018 · 61921fd · 61921fd
1 parent f883e52
commit 61921fd
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 1.2.3
+* UPD: images checking -- security improvement
+
 ## 1.2.2
 * UPD: package-lock was updated
 * UPD: was added new packages

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "powerbi-visuals-enhancedscatter",
-  "version": "1.2.2",
+  "version": "1.2.3",
   "description": "A few more properties were added to the existing scatter chart visual, including shapes as markers, background image support, and developer crosshairs for positioning elements onto an image background.",
   "repository": {
     "type": "git",
@@ -49,7 +49,7 @@
     "powerbi-visuals-utils-chartutils": "0.3.0",
     "powerbi-visuals-utils-formattingutils": "0.2.2",
     "powerbi-visuals-utils-colorutils": "0.2.2",
-    "powerbi-visuals-utils-dataviewutils": "1.1.1",
+    "powerbi-visuals-utils-dataviewutils": "1.4.1",
     "powerbi-visuals-utils-interactivityutils": "0.3.1",
     "powerbi-visuals-utils-testutils": "1.0.0",
     "powerbi-visuals-utils-tooltiputils": "0.3.2",

diff --git a/pbiviz.json b/pbiviz.json
@@ -4,7 +4,7 @@
     "displayName": "EnhancedScatter",
     "guid": "EnhancedScatterChart1443994985041",
     "visualClassName": "EnhancedScatterChart",
-    "version": "1.2.2",
+    "version": "1.2.3",
     "description": "A few more properties were added to the existing scatter chart visual, including shapes as markers, background image support, and developer crosshairs for positioning elements onto an image background.",
     "supportUrl": "http://community.powerbi.com",
     "gitHubUrl": "https://github.com/Microsoft/powerbi-visuals-enhancedscatter"

diff --git a/src/visual.ts b/src/visual.ts
@@ -38,6 +38,7 @@ module powerbi.extensibility.visual {
     import DataViewObjects = powerbi.extensibility.utils.dataview.DataViewObjects;
     import getMeasureIndexOfRole = powerbi.extensibility.utils.dataview.DataRoleHelper.getMeasureIndexOfRole;
     import getCategoryIndexOfRole = powerbi.extensibility.utils.dataview.DataRoleHelper.getCategoryIndexOfRole;
+    import ValidationHelper = powerbi.extensibility.utils.dataview.validationHelper;
 
     // powerbi.extensibility.utils.svg
     import svg = powerbi.extensibility.utils.svg;
@@ -1207,6 +1208,11 @@ module powerbi.extensibility.visual {
                         EnhancedScatterChart.getValueFromDataViewValueColumnById(measureShape, categoryIdx));
 
                     image = EnhancedScatterChart.getValueFromDataViewValueColumnById(measureImage, categoryIdx);
+
+                    if (image && !ValidationHelper.isImageUrlAllowed(image)) {
+                        image = null;
+                    }
+
                     rotation = EnhancedScatterChart.getNumberFromDataViewValueColumnById(measureRotation, categoryIdx);
                     backdrop = EnhancedScatterChart.getValueFromDataViewValueColumnById(measureBackdrop, categoryIdx);
                     xStart = EnhancedScatterChart.getValueFromDataViewValueColumnById(measureXStart, categoryIdx);
@@ -1607,7 +1613,6 @@ module powerbi.extensibility.visual {
             img.src = this.data.backdrop.url;
             img.onload = function () {
                 const imageElement: HTMLImageElement = this as HTMLImageElement;
-
                 if (that.oldBackdrop !== imageElement.src) {
                     that.render();
                     that.oldBackdrop = imageElement.src;
@@ -1787,10 +1792,13 @@ module powerbi.extensibility.visual {
                 }
             }
 
+            let isImageValid: boolean = ValidationHelper.isImageUrlAllowed(this.data.backdrop.url);
+
             // we have to do the above process again since changes are made to viewport.
             if (this.data.backdrop
                 && this.data.backdrop.show
-                && (this.data.backdrop.url !== undefined)) {
+                && (this.data.backdrop.url !== undefined)
+                && isImageValid) {
 
                 this.adjustViewportbyBackdrop();
 
@@ -1890,7 +1898,8 @@ module powerbi.extensibility.visual {
                 this.yAxisProperties,
                 tickLabelMargins,
                 chartHasAxisLabels,
-                axisLabels);
+                axisLabels,
+                isImageValid);
 
             this.updateAxis();
 
@@ -2420,10 +2429,11 @@ module powerbi.extensibility.visual {
             return elementUpdateSelection;
         }
 
-        private renderBackground(): void {
+        private renderBackground(isImageValid: boolean): void {
             if (this.data.backdrop
                 && this.data.backdrop.show
-                && (this.data.backdrop.url !== undefined)) {
+                && (this.data.backdrop.url !== undefined)
+                && isImageValid) {
 
                 this.backgroundGraphicsContext.attr({
                     "xlink:href": this.data.backdrop.url,
@@ -2445,13 +2455,15 @@ module powerbi.extensibility.visual {
             yAxis: IAxisProperties,
             tickLabelMargins: any,
             chartHasAxisLabels: boolean,
-            axisLabels: ChartAxesLabels): void {
+            axisLabels: ChartAxesLabels,
+            isImageValid: boolean): void {
 
             let bottomMarginLimit: number = this.bottomMarginLimit,
                 leftRightMarginLimit: number = this.leftRightMarginLimit,
                 duration: number = EnhancedScatterChart.AnimationDuration;
 
-            this.renderBackground();
+            this.renderBackground(isImageValid);
+
 
             // hide show x-axis here
             if (this.shouldRenderAxis(xAxis)) {

diff --git a/test/visualData.ts b/test/visualData.ts
@@ -77,9 +77,9 @@ module powerbi.extensibility.visual.test {
         public colorValues: string[] = ["red", "green", "blue"];
 
         public imageValues: string[] = [
-            "Microsoft_Access.png",
-            "Microsoft_OneNote.png",
-            "Microsoft_Outlook.png"
+            "https://somehost.someroothost/Microsoft_Access.png",
+            "https://somehost.someroothost/Microsoft_OneNote.png",
+            "https://somehost.someroothost/Microsoft_Outlook.png"
         ];
 
         public rotationValues: number[] = getRandomNumbers(this.valuesCategory.length, 100, 1000);

diff --git a/test/visualTest.ts b/test/visualTest.ts
@@ -457,7 +457,7 @@ module powerbi.extensibility.visual.test {
                 });
 
                 it("show", () => {
-                    (dataView.metadata.objects as any).backdrop.url = "https://test.url";
+                    (dataView.metadata.objects as any).backdrop.url = "https://test.url.jpg";
                     (dataView.metadata.objects as any).backdrop.show = true;
 
                     visualBuilder.updateFlushAllD3Transitions(dataView);
@@ -472,13 +472,22 @@ module powerbi.extensibility.visual.test {
                 });
 
                 it("url", () => {
-                    const url: string = "https://test.url";
+                    const url: string = "https://test.url.gif";
 
                     (dataView.metadata.objects as any).backdrop.url = url;
                     visualBuilder.updateFlushAllD3Transitions(dataView);
 
                     expect(visualBuilder.backdropImage.attr("href")).toBe(url);
                 });
+
+                it("invalid url", () => {
+                    const url: string = "https://test.url";
+
+                    (dataView.metadata.objects as any).backdrop.url = url;
+                    visualBuilder.updateFlushAllD3Transitions(dataView);
+
+                    expect(visualBuilder.backdropImage.attr("href")).toBeUndefined();
+                });
             });
 
             describe("Crosshair", () => {
@@ -842,6 +851,24 @@ module powerbi.extensibility.visual.test {
                         [EnhancedScatterChartData.ColumnImage]);
                 });
 
+                it("invalid images", () => {
+
+                    defaultDataViewBuilder.imageValues = [
+                        "Microsoft_Access.png",
+                        "Microsoft_OneNote.png",
+                        "Microsoft_Outlook.png"
+                    ];
+
+                    checkDataPointProperty(
+                        (dataPoint: EnhancedScatterChartDataPoint, index: number) => {
+                            expect(dataPoint.svgurl).toBeNull();
+                        },
+                        defaultDataViewBuilder,
+                        colorPalette,
+                        visualHost,
+                        [EnhancedScatterChartData.ColumnImage]);
+                });
+
                 it("rotate should be defined", () => {
                     checkDataPointProperty(
                         (dataPoint: EnhancedScatterChartDataPoint, index) => {