Copilot commented on code in PR #40621:
URL: https://github.com/apache/superset/pull/40621#discussion_r3337718034
##########
superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx:
##########
@@ -52,6 +53,46 @@ describe('getOverrideHtmlSchema', () => {
});
});
+describe('transformMarkdownLinkUri', () => {
+ test('should drop javascript: protocol links', () => {
+ expect(transformMarkdownLinkUri('javascript:alert(1)')).toEqual('');
+ });
+
+ test('should drop links with leading whitespace and mixed case', () => {
+ expect(transformMarkdownLinkUri(' JavaScript:alert(1)')).toEqual('');
+ });
+
+ test('should drop vbscript: protocol links', () => {
+ expect(transformMarkdownLinkUri('vbscript:msgbox(1)')).toEqual('');
+ });
Review Comment:
Add coverage for common obfuscation variants of `javascript:` (e.g. embedded
control characters like `\n` / `\t`). These are a frequent bypass technique for
simple `^javascript:` checks and should be part of the unit tests for the
exported transformer.
##########
superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown/SafeMarkdown.tsx:
##########
@@ -25,6 +25,17 @@ import remarkGfm from 'remark-gfm';
import { mergeWith } from 'lodash';
import { FeatureFlag, isFeatureEnabled } from '../../utils';
+// Reject link protocols that can execute script; allow everything else
+// (including the custom schemes supported since #26211).
+const DANGEROUS_LINK_PROTOCOL = /^\s*(?:javascript|vbscript|data):/i;
+
+export function transformMarkdownLinkUri(uri: string): string {
+ if (typeof uri !== 'string') {
+ return '';
+ }
+ return DANGEROUS_LINK_PROTOCOL.test(uri) ? '' : uri;
+}
Review Comment:
The current protocol check only matches exact `javascript:`, `vbscript:`, or
`data:` at the start (after leading whitespace). This can be bypassed by
inserting control characters/whitespace inside the scheme (e.g. `java\nscript:`
/ `java\tscript:`), which browsers may normalize to `javascript:` in `href`
values. Consider normalizing the URI (remove ASCII control chars + whitespace)
for the *detection* step before checking the scheme.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
