From a3e13b333e8b790a3466263f9a8c0c4fa5fc4d07 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Thu, 17 Jan 2019 11:06:47 +0100 Subject: [PATCH 1/4] fix: Text panel should re-render when panel mode is changed #14922 --- public/app/plugins/panel/text/module.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/public/app/plugins/panel/text/module.ts b/public/app/plugins/panel/text/module.ts index 08ab4cd2b96..ad4d07dbf63 100644 --- a/public/app/plugins/panel/text/module.ts +++ b/public/app/plugins/panel/text/module.ts @@ -33,11 +33,19 @@ export class TextPanelCtrl extends PanelCtrl { this.events.on('refresh', this.onRefresh.bind(this)); this.events.on('render', this.onRender.bind(this)); + const renderWhenChanged = (scope: any) => { + const { panel } = scope.ctrl; + return [ + panel.content, + panel.mode + ].join(); + }; + $scope.$watch( - 'ctrl.panel.content', + renderWhenChanged, _.throttle(() => { this.render(); - }, 1000) + }, 1000, {trailing: true}) ); } From 15d560a1c01f5bfb354f83183886881554026bb8 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Thu, 17 Jan 2019 14:44:52 +0100 Subject: [PATCH 2/4] feat: wip: Sanitize user input on text panel --- conf/defaults.ini | 1 + package.json | 3 ++- pkg/api/frontendsettings.go | 1 + pkg/setting/setting.go | 5 ++++- public/app/core/config.ts | 4 +++- public/app/core/utils/text.ts | 12 +++++++++++- public/app/plugins/panel/text/module.ts | 14 ++++++++++---- yarn.lock | 13 +++++++++++++ 8 files changed, 45 insertions(+), 8 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 7f61ac96870..1de2545251c 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -570,6 +570,7 @@ callback_url = [panels] enable_alpha = false +sanitize_input = true [enterprise] license_path = diff --git a/package.json b/package.json index 470101ff0c4..ec6420564eb 100644 --- a/package.json +++ b/package.json @@ -188,7 +188,8 @@ "slate-react": "^0.12.4", "tether": "^1.4.0", "tether-drop": "https://github.com/torkelo/drop/tarball/master", - "tinycolor2": "^1.4.1" + "tinycolor2": "^1.4.1", + "xss": "^1.0.3" }, "resolutions": { "caniuse-db": "1.0.30000772", diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 6d6cc708496..47ad9a04b27 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -166,6 +166,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *m.ReqContext) (map[string]interf "externalUserMngLinkUrl": setting.ExternalUserMngLinkUrl, "externalUserMngLinkName": setting.ExternalUserMngLinkName, "viewersCanEdit": setting.ViewersCanEdit, + "sanitizeInput": hs.Cfg.SanitizeInput, "buildInfo": map[string]interface{}{ "version": setting.BuildVersion, "commit": setting.BuildCommit, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 1417392fdf8..8048d19df13 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -18,7 +18,7 @@ import ( "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/util" - "gopkg.in/ini.v1" + ini "gopkg.in/ini.v1" ) type Scheme string @@ -90,6 +90,7 @@ var ( EmailCodeValidMinutes int DataProxyWhiteList map[string]bool DisableBruteForceLoginProtection bool + SanitizeInput bool // Snapshots ExternalSnapshotUrl string @@ -222,6 +223,7 @@ type Cfg struct { MetricsEndpointBasicAuthUsername string MetricsEndpointBasicAuthPassword string EnableAlphaPanels bool + SanitizeInput bool EnterpriseLicensePath string } @@ -709,6 +711,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { panels := iniFile.Section("panels") cfg.EnableAlphaPanels = panels.Key("enable_alpha").MustBool(false) + cfg.SanitizeInput = panels.Key("sanitize_input").MustBool(true) cfg.readSessionConfig() cfg.readSmtpSettings() diff --git a/public/app/core/config.ts b/public/app/core/config.ts index 26f31ffcf54..3c7966850fa 100644 --- a/public/app/core/config.ts +++ b/public/app/core/config.ts @@ -35,8 +35,9 @@ export class Settings { loginHint: any; loginError: any; viewersCanEdit: boolean; + sanitizeInput: boolean; - constructor(options) { + constructor(options: Settings) { const defaults = { datasources: {}, windowTitlePrefix: 'Grafana - ', @@ -52,6 +53,7 @@ export class Settings { isEnterprise: false, }, viewersCanEdit: false, + sanitizeInput: true }; _.extend(this, defaults, options); diff --git a/public/app/core/utils/text.ts b/public/app/core/utils/text.ts index 4e948116dba..9f4f1c41716 100644 --- a/public/app/core/utils/text.ts +++ b/public/app/core/utils/text.ts @@ -1,4 +1,5 @@ import { TextMatch } from 'app/types/explore'; +import xss from 'xss'; /** * Adapt findMatchesInText for react-highlight-words findChunks handler. @@ -22,7 +23,7 @@ export function findMatchesInText(haystack: string, needle: string): TextMatch[] } const matches = []; const cleaned = cleanNeedle(needle); - let regexp; + let regexp: RegExp; try { regexp = new RegExp(`(?:${cleaned})`, 'g'); } catch (error) { @@ -42,3 +43,12 @@ export function findMatchesInText(haystack: string, needle: string): TextMatch[] }); return matches; } + +export function sanitize (unsanitizedString: string): string { + try { + return xss(unsanitizedString); + } catch (error) { + console.log('String could not be sanitized', unsanitizedString); + return unsanitizedString; + } +} diff --git a/public/app/plugins/panel/text/module.ts b/public/app/plugins/panel/text/module.ts index ad4d07dbf63..7edd9c3a87e 100644 --- a/public/app/plugins/panel/text/module.ts +++ b/public/app/plugins/panel/text/module.ts @@ -1,6 +1,8 @@ import _ from 'lodash'; import { PanelCtrl } from 'app/plugins/sdk'; import Remarkable from 'remarkable'; +import { sanitize } from 'app/core/utils/text'; +import config from 'app/core/config'; const defaultContent = ` # Title @@ -44,8 +46,9 @@ export class TextPanelCtrl extends PanelCtrl { $scope.$watch( renderWhenChanged, _.throttle(() => { + console.log('this.render', new Date()); this.render(); - }, 1000, {trailing: true}) + }, 2000, {trailing: true, leading: true}) ); } @@ -70,7 +73,7 @@ export class TextPanelCtrl extends PanelCtrl { this.renderingCompleted(); } - renderText(content) { + renderText(content: string) { content = content .replace(/&/g, '&') .replace(/>/g, '>') @@ -79,7 +82,7 @@ export class TextPanelCtrl extends PanelCtrl { this.updateContent(content); } - renderMarkdown(content) { + renderMarkdown(content: string) { if (!this.remarkable) { this.remarkable = new Remarkable(); } @@ -89,7 +92,10 @@ export class TextPanelCtrl extends PanelCtrl { }); } - updateContent(html) { + updateContent(html: string) { + const { sanitizeInput } = config; + html = sanitizeInput ? sanitize(html) : html; + console.log('html', html); try { this.content = this.$sce.trustAsHtml(this.templateSrv.replace(html, this.panel.scopedVars)); } catch (e) { diff --git a/yarn.lock b/yarn.lock index 62a059cffec..25467969e35 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3560,6 +3560,11 @@ cssesc@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/cssesc/-/cssesc-0.1.0.tgz#c814903e45623371a0477b40109aaafbeeaddbb4" +cssfilter@0.0.10: + version "0.0.10" + resolved "https://registry.yarnpkg.com/cssfilter/-/cssfilter-0.0.10.tgz#c6d2672632a2e5c83e013e6864a42ce8defd20ae" + integrity sha1-xtJnJjKi5cg+AT5oZKQs6N79IK4= + cssnano@^3.10.0: version "3.10.0" resolved "https://registry.yarnpkg.com/cssnano/-/cssnano-3.10.0.tgz#4f38f6cea2b9b17fa01490f23f1dc68ea65c1c38" @@ -13344,6 +13349,14 @@ xregexp@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/xregexp/-/xregexp-4.0.0.tgz#e698189de49dd2a18cc5687b05e17c8e43943020" +xss@^1.0.3: + version "1.0.3" + resolved "https://registry.yarnpkg.com/xss/-/xss-1.0.3.tgz#d04bd2558fd6c29c46113824d5e8b2a910054e23" + integrity sha512-LTpz3jXPLUphMMmyufoZRSKnqMj41OVypZ8uYGzvjkMV9C1EdACrhQl/EM8Qfh5htSAuMIQFOejmKAZGkJfaCg== + dependencies: + commander "^2.9.0" + cssfilter "0.0.10" + xtend@^4.0.0, xtend@~4.0.0, xtend@~4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.1.tgz#a5c6d532be656e23db820efb943a1f04998d63af" From 1ed35f3dc117db7feed6d8c2a950e22713c17d1b Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Mon, 21 Jan 2019 16:13:26 +0100 Subject: [PATCH 3/4] chore: Reverse sanitize variable so it defaults to false --- conf/defaults.ini | 2 +- pkg/api/frontendsettings.go | 2 +- pkg/setting/setting.go | 6 +++--- public/app/core/config.ts | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 1de2545251c..b0de259de19 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -570,7 +570,7 @@ callback_url = [panels] enable_alpha = false -sanitize_input = true +disable_sanitize_input = false [enterprise] license_path = diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 47ad9a04b27..adf7e83325e 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -166,7 +166,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *m.ReqContext) (map[string]interf "externalUserMngLinkUrl": setting.ExternalUserMngLinkUrl, "externalUserMngLinkName": setting.ExternalUserMngLinkName, "viewersCanEdit": setting.ViewersCanEdit, - "sanitizeInput": hs.Cfg.SanitizeInput, + "disableSanitizeInput": hs.Cfg.DisableSanitizeInput, "buildInfo": map[string]interface{}{ "version": setting.BuildVersion, "commit": setting.BuildCommit, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 8048d19df13..1f9db6fbb78 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -90,7 +90,7 @@ var ( EmailCodeValidMinutes int DataProxyWhiteList map[string]bool DisableBruteForceLoginProtection bool - SanitizeInput bool + DisableSanitizeInput bool // Snapshots ExternalSnapshotUrl string @@ -223,7 +223,7 @@ type Cfg struct { MetricsEndpointBasicAuthUsername string MetricsEndpointBasicAuthPassword string EnableAlphaPanels bool - SanitizeInput bool + DisableSanitizeInput bool EnterpriseLicensePath string } @@ -711,7 +711,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { panels := iniFile.Section("panels") cfg.EnableAlphaPanels = panels.Key("enable_alpha").MustBool(false) - cfg.SanitizeInput = panels.Key("sanitize_input").MustBool(true) + cfg.DisableSanitizeInput = panels.Key("sanitize_input_disabled").MustBool(false) cfg.readSessionConfig() cfg.readSmtpSettings() diff --git a/public/app/core/config.ts b/public/app/core/config.ts index 3c7966850fa..ce7f0fcfe50 100644 --- a/public/app/core/config.ts +++ b/public/app/core/config.ts @@ -35,7 +35,7 @@ export class Settings { loginHint: any; loginError: any; viewersCanEdit: boolean; - sanitizeInput: boolean; + disableSanitizeInput: boolean; constructor(options: Settings) { const defaults = { @@ -53,7 +53,7 @@ export class Settings { isEnterprise: false, }, viewersCanEdit: false, - sanitizeInput: true + disableSanitizeInput: false }; _.extend(this, defaults, options); From 08dc5a4f9783a0ca0d9c6b703a883cbab1369c04 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Mon, 21 Jan 2019 16:34:59 +0100 Subject: [PATCH 4/4] chore: Remove logging and use the updated config param --- public/app/plugins/panel/text/module.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/public/app/plugins/panel/text/module.ts b/public/app/plugins/panel/text/module.ts index 7edd9c3a87e..7d2a0ab0dd0 100644 --- a/public/app/plugins/panel/text/module.ts +++ b/public/app/plugins/panel/text/module.ts @@ -46,9 +46,8 @@ export class TextPanelCtrl extends PanelCtrl { $scope.$watch( renderWhenChanged, _.throttle(() => { - console.log('this.render', new Date()); this.render(); - }, 2000, {trailing: true, leading: true}) + }, 100) ); } @@ -93,9 +92,8 @@ export class TextPanelCtrl extends PanelCtrl { } updateContent(html: string) { - const { sanitizeInput } = config; - html = sanitizeInput ? sanitize(html) : html; - console.log('html', html); + const { disableSanitizeInput } = config; + html = disableSanitizeInput ? html : sanitize(html); try { this.content = this.$sce.trustAsHtml(this.templateSrv.replace(html, this.panel.scopedVars)); } catch (e) {