Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F66017217
T404392-3.patch
gengh (Genoveva Galarza)
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Authored By
gengh
Sep 12 2025, 11:31 AM
2025-09-12 11:31:18 (UTC+0)
Size
17 KB
Referenced Files
None
Subscribers
None
T404392-3.patch
View Options
From 8993bb77fedf6908ea200a7c8752e40ee4ef0536 Mon Sep 17 00:00:00 2001
From: gengh <ggalarzaheredero@wikimedia.org>
Date: Fri, 12 Sep 2025 12:50:58 +0200
Subject: [PATCH] SECURITY: Do not let error type labels or arguments return
HTML ever
Bug: T404392
Change-Id: I996fa2f9ec5112978296ad8f357176123f8f692b
---
resources/.eslintrc.json | 1 +
.../components/base/ZObjectSelector.vue | 3 +--
.../components/types/ZArgumentReference.vue | 3 +--
.../components/types/ZCode.vue | 6 ++----
.../types/ZObjectStringRenderer.vue | 3 +--
.../visualeditor/FunctionInputField.vue | 3 +--
.../fields/FunctionInputParser.vue | 8 ++++----
.../FunctionMetadataDialog.vue | 15 +++++++++------
.../widgets/publish/PublishDialog.vue | 4 ++--
.../ext.wikilambda.app/mixins/errorMixin.js | 9 ++++++---
.../ext.wikilambda.app/utils/errorUtils.js | 12 ++++++++++++
.../FunctionMetadataDialog.test.js | 18 ++++++++++++++++++
.../widgets/publish/PublishDialog.test.js | 19 +++++++++++++++++++
tests/jest/fixtures/metadata.js | 18 +++++++++++++++++-
14 files changed, 94 insertions(+), 28 deletions(-)
diff --git a/resources/.eslintrc.json b/resources/.eslintrc.json
index 70c9dce1..15e61dc2 100644
--- a/resources/.eslintrc.json
+++ b/resources/.eslintrc.json
@@ -42,6 +42,7 @@
"error",
"kebab-case"
],
+ "vue/no-v-html": "error",
"vue/no-undef-properties": "off",
"vue/require-explicit-emits": "off",
"vue/no-unsupported-features": [
diff --git a/resources/ext.wikilambda.app/components/base/ZObjectSelector.vue b/resources/ext.wikilambda.app/components/base/ZObjectSelector.vue
index 70bcaad0..bb6f4ee6 100644
--- a/resources/ext.wikilambda.app/components/base/ZObjectSelector.vue
+++ b/resources/ext.wikilambda.app/components/base/ZObjectSelector.vue
@@ -51,8 +51,7 @@
:type="error.type"
:inline="true"
>
- <!-- eslint-disable vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</div>
</span>
diff --git a/resources/ext.wikilambda.app/components/types/ZArgumentReference.vue b/resources/ext.wikilambda.app/components/types/ZArgumentReference.vue
index d07ebcf7..37fc27ae 100644
--- a/resources/ext.wikilambda.app/components/types/ZArgumentReference.vue
+++ b/resources/ext.wikilambda.app/components/types/ZArgumentReference.vue
@@ -34,8 +34,7 @@
:type="error.type"
:inline="true"
>
- <!-- eslint-disable vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</div>
</div>
diff --git a/resources/ext.wikilambda.app/components/types/ZCode.vue b/resources/ext.wikilambda.app/components/types/ZCode.vue
index 69f8c973..30ac3cb0 100644
--- a/resources/ext.wikilambda.app/components/types/ZCode.vue
+++ b/resources/ext.wikilambda.app/components/types/ZCode.vue
@@ -42,8 +42,7 @@
:type="error.type"
:inline="true"
>
- <!-- eslint-disable-next-line vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</template>
</wl-key-value-block>
@@ -79,8 +78,7 @@
:type="error.type"
:inline="true"
>
- <!-- eslint-disable-next-line vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</template>
</wl-key-value-block>
diff --git a/resources/ext.wikilambda.app/components/types/ZObjectStringRenderer.vue b/resources/ext.wikilambda.app/components/types/ZObjectStringRenderer.vue
index b69a50c3..0c6ca02f 100644
--- a/resources/ext.wikilambda.app/components/types/ZObjectStringRenderer.vue
+++ b/resources/ext.wikilambda.app/components/types/ZObjectStringRenderer.vue
@@ -31,8 +31,7 @@
class="ext-wikilambda-app-object-string-renderer__error"
>
<cdx-message :type="fieldErrors[0].type" :inline="true">
- <!-- eslint-disable vue/no-v-html -->
- <span v-html="getErrorMessage( fieldErrors[0] )"></span>
+ <span>{{ getErrorMessage( fieldErrors[0] ) }}</span>
</cdx-message>
<a v-if="showExamplesLink" @click="openExamplesDialog">
{{ $i18n( 'wikilambda-string-renderer-examples-title' ).text() }}
diff --git a/resources/ext.wikilambda.app/components/visualeditor/FunctionInputField.vue b/resources/ext.wikilambda.app/components/visualeditor/FunctionInputField.vue
index 74acbb8a..b83cc484 100644
--- a/resources/ext.wikilambda.app/components/visualeditor/FunctionInputField.vue
+++ b/resources/ext.wikilambda.app/components/visualeditor/FunctionInputField.vue
@@ -32,8 +32,7 @@
</span>
</template>
<template v-if="showValidation && !!errorMessage" #error>
- <!-- eslint-disable-next-line vue/no-v-html -->
- <span v-html="errorMessage"></span>
+ <div>{{ errorMessage }}</div>
</template>
</cdx-field>
</template>
diff --git a/resources/ext.wikilambda.app/components/visualeditor/fields/FunctionInputParser.vue b/resources/ext.wikilambda.app/components/visualeditor/fields/FunctionInputParser.vue
index 0e356fcf..c2d0e1fa 100644
--- a/resources/ext.wikilambda.app/components/visualeditor/fields/FunctionInputParser.vue
+++ b/resources/ext.wikilambda.app/components/visualeditor/fields/FunctionInputParser.vue
@@ -128,7 +128,7 @@ module.exports = exports = defineComponent( {
*
* @return {string}
*/
- getErrorMessage: function () {
+ fallbackErrorMsg: function () {
return this.$i18n( 'wikilambda-visualeditor-wikifunctionscall-error-parser', this.inputType ).parse();
},
/**
@@ -186,10 +186,10 @@ module.exports = exports = defineComponent( {
// * get error from metadata object
// * reject with error message
const metadata = data.response[ Constants.Z_RESPONSEENVELOPE_METADATA ];
- this.setErrorMessageCallback( metadata, this.getErrorMessage, reject );
+ this.setErrorMessageCallback( metadata, this.fallbackErrorMsg, reject );
} else if ( this.typeToString( this.getZObjectType( response ) ) !== this.inputType ) {
// Parser return unexpected type: reject with error message
- reject( this.getErrorMessage );
+ reject( this.fallbackErrorMsg );
} else {
// Success: Resolve the promise
resolve();
@@ -199,7 +199,7 @@ module.exports = exports = defineComponent( {
if ( error.code === 'abort' ) {
reject( error.code );
}
- reject( this.getErrorMessage );
+ reject( this.fallbackErrorMsg );
} );
} );
},
diff --git a/resources/ext.wikilambda.app/components/widgets/function-evaluator/FunctionMetadataDialog.vue b/resources/ext.wikilambda.app/components/widgets/function-evaluator/FunctionMetadataDialog.vue
index d41f1540..2029c373 100644
--- a/resources/ext.wikilambda.app/components/widgets/function-evaluator/FunctionMetadataDialog.vue
+++ b/resources/ext.wikilambda.app/components/widgets/function-evaluator/FunctionMetadataDialog.vue
@@ -44,12 +44,12 @@
class="ext-wikilambda-metadata-dialog-errors"
:type="error.type"
>
- <!-- eslint-disable vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</div>
<div v-else class="ext-wikilambda-app-function-metadata-dialog__body">
<cdx-message v-if="hasMetadataErrors">
+ <!-- eslint-disable vue/no-v-html -->
<div v-html="$i18n( 'wikilambda-functioncall-metadata-errors-debug-hint' ).parse()"></div>
</cdx-message>
<cdx-field
@@ -134,7 +134,7 @@ const errorMixin = require( '../../../mixins/errorMixin.js' );
const LabelData = require( '../../../store/classes/LabelData.js' );
const metadataMixin = require( '../../../mixins/metadataMixin.js' );
const useMainStore = require( '../../../store/index.js' );
-const { extractErrorData } = require( '../../../utils/errorUtils.js' );
+const { extractErrorData, cleanUpForHTML } = require( '../../../utils/errorUtils.js' );
const { extractZIDs } = require( '../../../utils/schemata.js' );
const typeMixin = require( '../../../mixins/typeMixin.js' );
const icons = require( '../../../../lib/icons.json' );
@@ -558,7 +558,7 @@ module.exports = exports = defineComponent( {
const args = [];
for ( const arg of errorData.stringArgs ) {
const key = this.getLabelData( arg.key ).label;
- const value = this.$i18n( 'quotation-marks', arg.value ).text();
+ const value = this.$i18n( 'quotation-marks', cleanUpForHTML( arg.value ) ).text();
args.push( `${ key }${ colon }${ value }` );
}
const argblock = this.$i18n( 'parentheses', args.join( comma ) ).text();
@@ -666,8 +666,11 @@ module.exports = exports = defineComponent( {
const list = [];
for ( const arg of data.stringArgs ) {
const key = this.getLabelData( arg.key );
- const keySpan = `<span dir="${ key.langDir }" lang="${ key.langCode }">${ key.labelOrUntitled }</span>`;
- list.push( `<li>${ keySpan }: "${ arg.value }"</li>` );
+ // SECURITY: Escape any HTML in the argument value
+ const escapedLabel = cleanUpForHTML( key.labelOrUntitled );
+ const escapedArg = cleanUpForHTML( arg.value );
+ const keySpan = `<span dir="${ key.langDir }" lang="${ key.langCode }">${ escapedLabel }</span>`;
+ list.push( `<li>${ keySpan }: "${ escapedArg }"</li>` );
}
return `<ul>${ list.join( '' ) }</ul>`;
}
diff --git a/resources/ext.wikilambda.app/components/widgets/publish/PublishDialog.vue b/resources/ext.wikilambda.app/components/widgets/publish/PublishDialog.vue
index cf2b7f16..8244b3b8 100644
--- a/resources/ext.wikilambda.app/components/widgets/publish/PublishDialog.vue
+++ b/resources/ext.wikilambda.app/components/widgets/publish/PublishDialog.vue
@@ -30,8 +30,7 @@
class="ext-wikilambda-app-publish-dialog__error"
:type="error.type"
>
- <!-- eslint-disable vue/no-v-html -->
- <div v-html="getErrorMessage( error )"></div>
+ <div>{{ getErrorMessage( error ) }}</div>
</cdx-message>
</div>
@@ -56,6 +55,7 @@
<!-- Legal text -->
<template #footer-text>
+ <!-- eslint-disable vue/no-v-html -->
<div
class="ext-wikilambda-app-publish-dialog__legal-text"
v-html="legalText"
diff --git a/resources/ext.wikilambda.app/mixins/errorMixin.js b/resources/ext.wikilambda.app/mixins/errorMixin.js
index 8afa30da..8c5b27e4 100644
--- a/resources/ext.wikilambda.app/mixins/errorMixin.js
+++ b/resources/ext.wikilambda.app/mixins/errorMixin.js
@@ -11,7 +11,7 @@ const { mapActions, mapState } = require( 'pinia' );
const Constants = require( '../Constants.js' );
const useMainStore = require( '../store/index.js' );
-const { extractErrorData } = require( '../utils/errorUtils.js' );
+const { extractErrorData, cleanUpForHTML } = require( '../utils/errorUtils.js' );
const { getValueFromCanonicalZMap } = require( '../utils/schemata.js' );
module.exports = exports = {
@@ -34,15 +34,18 @@ module.exports = exports = {
/**
* Returns the translated message for a given error code.
- * Error messages can have html tags.
+ * Raw error messages cannot have HTML tags, as they are escaped.
*
* @memberof module:ext.wikilambda.app.mixins.errorMixin
* @param {Object} error
* @return {string}
*/
getErrorMessage: function ( error ) {
+ if ( error.message ) {
+ return cleanUpForHTML( error.message );
+ }
// eslint-disable-next-line mediawiki/msg-doc
- return error.message || this.$i18n( error.code ).parse();
+ return this.$i18n( error.code ).parse();
},
/**
diff --git a/resources/ext.wikilambda.app/utils/errorUtils.js b/resources/ext.wikilambda.app/utils/errorUtils.js
index d1e78100..c0776a03 100644
--- a/resources/ext.wikilambda.app/utils/errorUtils.js
+++ b/resources/ext.wikilambda.app/utils/errorUtils.js
@@ -75,6 +75,18 @@ const errorUtils = {
// Canonicalize whole error (just in case) before extracting its data
const canonicalError = hybridToCanonical( zobject );
return extractNestedErrors( canonicalError );
+ },
+
+ /**
+ * Sanitize a string for safe HTML rendering.
+ *
+ * @param {string} input
+ * @return {string}
+ */
+ cleanUpForHTML: ( input ) => {
+ const div = document.createElement( 'div' );
+ div.appendChild( document.createTextNode( input ) );
+ return div.innerHTML;
}
};
diff --git a/tests/jest/components/widgets/function-evaluator/FunctionMetadataDialog.test.js b/tests/jest/components/widgets/function-evaluator/FunctionMetadataDialog.test.js
index d16eff45..da0f1641 100644
--- a/tests/jest/components/widgets/function-evaluator/FunctionMetadataDialog.test.js
+++ b/tests/jest/components/widgets/function-evaluator/FunctionMetadataDialog.test.js
@@ -300,6 +300,19 @@ describe( 'dialog', () => {
expect( message[ 0 ].text() ).toContain( 'Something not working? Try Wikifunctions.Debug to trace your code.' );
} );
+ it( 'renders the error section, but escapes bad stuff', () => {
+ const wrapper = mount( FunctionMetadataDialog, {
+ props: { open: true, metadata: metadata.metadataMaliciousError }
+ } );
+ const sections = wrapper.findAllComponents( { name: 'cdx-accordion' } );
+ expect( sections.length ).toBe( 1 );
+ const section = sections[ 0 ];
+
+ // Check header
+ expect( section.find( '.cdx-accordion__header__title' ).text() ).toBe( '{{PLURAL:$1|Error|Errors}}' );
+ expect( section.find( '.cdx-accordion__header__description' ).text() ).toBe( 'Z500 (Z500K1: "<button onmouseover="window.location = \'//www.example.com\'">")' );
+ } );
+
it( 'renders the expected/actual values even if the error data is missing', () => {
const wrapper = mount( FunctionMetadataDialog, {
props: { open: true, metadata: metadata.metadataDifferButNoErrors }
@@ -377,6 +390,7 @@ describe( 'dialog', () => {
const selector = wrapper.findComponent( { name: 'cdx-select' } );
expect( selector.html() ).toContain( 'ext-wikilambda-app-function-metadata-dialog__selected--pass' );
expect( selector.html() ).not.toContain( 'ext-wikilambda-app-function-metadata-dialog__selected--fail' );
+ expect( selector.html() ).toContain( '<bdi>If (Echo (true), Echo ("is true"), "is false")</bdi>' );
// Select second child
const selectedId = '0-1';
@@ -386,10 +400,14 @@ describe( 'dialog', () => {
// Selector class has changed from pass to fail
expect( selector.html() ).toContain( 'ext-wikilambda-app-function-metadata-dialog__selected--fail' );
expect( selector.html() ).not.toContain( 'ext-wikilambda-app-function-metadata-dialog__selected--pass' );
+ expect( selector.html() ).toContain( '<bdi>Echo</bdi>' );
// Metadata body is now reflecting a different metadata set
sections = wrapper.findAllComponents( { name: 'cdx-accordion' } );
expect( sections.length ).toBe( 3 );
+
+ // SECURITY: Ensure malicious HTML in error message values is also escaped
+ expect( sections[ 0 ].html() ).toContain( '"some error in child function call: <button onmouseover="window.location = \'//www.example.com\'">"' );
} );
} );
} );
diff --git a/tests/jest/components/widgets/publish/PublishDialog.test.js b/tests/jest/components/widgets/publish/PublishDialog.test.js
index e5eeb978..caf44b4c 100644
--- a/tests/jest/components/widgets/publish/PublishDialog.test.js
+++ b/tests/jest/components/widgets/publish/PublishDialog.test.js
@@ -91,6 +91,25 @@ describe( 'Publish Dialog', () => {
expect( messages[ 1 ].text() ).toBe( 'Unable to complete request. Please try again.' );
} );
+ it( 'renders escaped page errors', () => {
+ const errors = [ {
+ message: "<button onmouseover=\"window.location = '//www.example.com'\">",
+ code: undefined,
+ type: Constants.ERROR_TYPES.ERROR
+ } ];
+ store.getErrors = createGettersWithFunctionsMock( errors );
+
+ const wrapper = mount( PublishDialog, {
+ props: { showDialog: true },
+ global: { stubs: dialogGlobalStubs }
+ } );
+
+ const messages = wrapper.findAllComponents( { name: 'cdx-message' } );
+ expect( messages.length ).toBe( 1 );
+ expect( messages[ 0 ].props( 'type' ) ).toBe( 'error' );
+ expect( messages[ 0 ].text() ).toBe( '<button onmouseover="window.location = \'//www.example.com\'">' );
+ } );
+
it( 'closes the dialog when click cancel button', () => {
const wrapper = mount( PublishDialog, {
props: { showDialog: true },
diff --git a/tests/jest/fixtures/metadata.js b/tests/jest/fixtures/metadata.js
index 8972fdc5..5cc83be7 100644
--- a/tests/jest/fixtures/metadata.js
+++ b/tests/jest/fixtures/metadata.js
@@ -75,7 +75,7 @@ const metadataChild2 = convertSetToMap( {
Z7K1: "Z885",
Z885K1: "Z500"
},
- Z500K1: "some error in child function call",
+ Z500K1: "some error in child function call: <button onmouseover=\"window.location = '//www.example.com'\">",
}
},
// duration:
@@ -170,6 +170,21 @@ const metadataErrors = convertSetToMap( {
actualTestResult: 'CBA'
} );
+const metadataMaliciousError = convertSetToMap( {
+ errors: {
+ Z1K1: "Z5",
+ Z5K1: "Z500",
+ Z5K2: {
+ Z1K1: {
+ Z1K1: "Z7",
+ Z7K1: "Z885",
+ Z885K1: "Z500"
+ },
+ Z500K1: "<button onmouseover=\"window.location = '//www.example.com'\">"
+ }
+ }
+} );
+
const metadataDifferButNoErrors = convertSetToMap( {
expectedTestResult: 'ABC',
actualTestResult: 'CBA'
@@ -178,6 +193,7 @@ const metadataDifferButNoErrors = convertSetToMap( {
module.exports = {
metadataBasic,
metadataErrors,
+ metadataMaliciousError,
metadataEmpty,
metadataNested,
metadataDifferButNoErrors,
--
2.25.1
File Metadata
Details
Attached
Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
21797121
Default Alt Text
T404392-3.patch (17 KB)
Attached To
Mode
T404392: Arbitrary HTML injection through error display on Wikifunctions
Attached
Detach File
Event Timeline
Log In to Comment