Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Fixing XSS vulnerability in toast messages #10329

Merged
merged 10 commits into from
Aug 8, 2024
3 changes: 2 additions & 1 deletion packages/editor-ui/src/components/WorkflowActivator.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useWorkflowsStore } from '@/stores/workflows.store';
import { getActivatableTriggerNodes } from '@/utils/nodeTypesUtils';
import { computed } from 'vue';
import { useI18n } from '@/composables/useI18n';
import { sanitizeHtml } from '@/utils/htmlUtils';

const props = defineProps<{ workflowActive: boolean; workflowId: string }>();
const { showMessage } = useToast();
Expand Down Expand Up @@ -71,7 +72,7 @@ async function displayActivationError() {

showMessage({
title: i18n.baseText('workflowActivator.showMessage.displayActivationError.title'),
message: errorMessage,
message: sanitizeHtml(errorMessage),
netroy marked this conversation as resolved.
Show resolved Hide resolved
type: 'warning',
duration: 0,
dangerouslyUseHTMLString: true,
Expand Down
3 changes: 2 additions & 1 deletion packages/editor-ui/src/composables/useExecutionDebugging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useUIStore } from '@/stores/ui.store';
import { useTelemetry } from './useTelemetry';
import { useRootStore } from '@/stores/root.store';
import { isFullExecutionResponse } from '@/utils/typeGuards';
import { sanitizeHtml } from '@/utils/htmlUtils';

export const useExecutionDebugging = () => {
const telemetry = useTelemetry();
Expand Down Expand Up @@ -61,7 +62,7 @@ export const useExecutionDebugging = () => {
h(
'ul',
{ class: 'mt-l ml-l' },
matchingPinnedNodeNames.map((name) => h('li', name)),
matchingPinnedNodeNames.map((name) => h('li', sanitizeHtml(name))),
),
]);

Expand Down
2 changes: 0 additions & 2 deletions packages/editor-ui/src/composables/usePushConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
}),
type: 'error',
duration: 0,
dangerouslyUseHTMLString: true,
});
} else {
let title: string;
Expand All @@ -438,7 +437,6 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
message: runDataExecutedErrorMessage,
netroy marked this conversation as resolved.
Show resolved Hide resolved
type: 'error',
duration: 0,
dangerouslyUseHTMLString: true,
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/composables/useToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface NotificationErrorWithNodeAndDescription extends ApplicationErro
}

const messageDefaults: Partial<Omit<NotificationOptions, 'message'>> = {
dangerouslyUseHTMLString: true,
dangerouslyUseHTMLString: false,
netroy marked this conversation as resolved.
Show resolved Hide resolved
position: 'bottom-right',
};

Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,8 @@ export const ALLOWED_HTML_TAGS = [
'mark',
];

export const HREF_PROTOCOL_BLACKLIST = ['javascript:', 'vbscript:', 'data:'];

export const CLOUD_CHANGE_PLAN_PAGE = window.location.host.includes('stage-app.n8n.cloud')
? 'https://stage-app.n8n.cloud/account/change-plan'
: 'https://app.n8n.cloud/account/change-plan';
Expand Down
6 changes: 5 additions & 1 deletion packages/editor-ui/src/utils/htmlUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import xss, { friendlyAttrValue } from 'xss';
import { ALLOWED_HTML_ATTRIBUTES, ALLOWED_HTML_TAGS } from '@/constants';
import { ALLOWED_HTML_ATTRIBUTES, ALLOWED_HTML_TAGS, HREF_PROTOCOL_BLACKLIST } from '@/constants';

/*
Constants and utility functions that help in HTML, CSS and DOM manipulation
Expand All @@ -18,6 +18,10 @@ export function sanitizeHtml(dirtyHtml: string) {
}

if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) {
// href is allowed but we need to sanitize certain protocols
if (name === 'href' && HREF_PROTOCOL_BLACKLIST.includes(`${value.split(':')[0]}:`)) {
netroy marked this conversation as resolved.
Show resolved Hide resolved
return '';
}
return `${name}="${friendlyAttrValue(value)}"`;
}

Expand Down