mirror of
https://github.com/duckietm/Nitro-V3.git
synced 2026-06-19 15:06:20 +00:00
fix(security): harden external-link opening (protocol allow-list + noopener)
URLs reached window.open from user/server-controlled content without a protocol check or noopener, allowing reverse-tabnabbing and (for the chat link handler) a javascript:/data: href running in our origin. - add isSafeExternalUrl() (http/https only) + tests; gate the chat link opener (useOnClickChat) and external photo opener with it - SanitizeHtml: afterSanitizeAttributes hook forces rel="noopener noreferrer" on any target=_blank anchor (overrides attacker-supplied rel) - add noopener,noreferrer to the remaining window.open(_blank) sites (YouTube share, external photo, guide forum link); drop a stray console.log
This commit is contained in:
@@ -86,3 +86,19 @@ describe('SanitizeHtml — preserves the markup the chat/profile UI relies on',
|
|||||||
expect(parse('a<br />b').querySelectorAll('br').length).toBe(1);
|
expect(parse('a<br />b').querySelectorAll('br').length).toBe(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('SanitizeHtml — link safety', () =>
|
||||||
|
{
|
||||||
|
it('forces rel="noopener noreferrer" on a target=_blank anchor', () =>
|
||||||
|
{
|
||||||
|
const a = parse('<a href="https://example.com" target="_blank">x</a>').querySelector('a');
|
||||||
|
expect(a).not.toBeNull();
|
||||||
|
expect(a?.getAttribute('rel')).toBe('noopener noreferrer');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('overrides an attacker-supplied rel on a target=_blank anchor', () =>
|
||||||
|
{
|
||||||
|
const a = parse('<a href="https://example.com" target="_blank" rel="opener">x</a>').querySelector('a');
|
||||||
|
expect(a?.getAttribute('rel')).toBe('noopener noreferrer');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,10 +1,23 @@
|
|||||||
import DOMPurify from 'dompurify';
|
import DOMPurify from 'dompurify';
|
||||||
|
|
||||||
|
// Any link that opens a new browsing context gets a safe rel so it cannot
|
||||||
|
// reverse-tabnab the opener. Registered once at module load; applies to every
|
||||||
|
// SanitizeHtml() call (and overrides any attacker-supplied rel).
|
||||||
|
DOMPurify.addHook('afterSanitizeAttributes', node =>
|
||||||
|
{
|
||||||
|
const element = node as Element;
|
||||||
|
|
||||||
|
if((element.tagName === 'A') && element.getAttribute('target'))
|
||||||
|
{
|
||||||
|
element.setAttribute('rel', 'noopener noreferrer');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
export const SanitizeHtml = (html: string): string =>
|
export const SanitizeHtml = (html: string): string =>
|
||||||
{
|
{
|
||||||
return DOMPurify.sanitize(html, {
|
return DOMPurify.sanitize(html, {
|
||||||
ALLOWED_TAGS: [ 'b', 'i', 'u', 'br', 'span', 'div', 'p', 'a', 'strong', 'em', 'img' ],
|
ALLOWED_TAGS: [ 'b', 'i', 'u', 'br', 'span', 'div', 'p', 'a', 'strong', 'em', 'img' ],
|
||||||
ALLOWED_ATTR: [ 'href', 'target', 'class', 'style', 'src', 'alt' ],
|
ALLOWED_ATTR: [ 'href', 'target', 'class', 'style', 'src', 'alt', 'rel' ],
|
||||||
ALLOW_DATA_ATTR: false
|
ALLOW_DATA_ATTR: false
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ export * from './PrefixUtils';
|
|||||||
export * from './ProductImageUtility';
|
export * from './ProductImageUtility';
|
||||||
export * from './Randomizer';
|
export * from './Randomizer';
|
||||||
export * from './RememberLogin';
|
export * from './RememberLogin';
|
||||||
|
export * from './isSafeExternalUrl';
|
||||||
export * from './RoomChatFormatter';
|
export * from './RoomChatFormatter';
|
||||||
export * from './SanitizeHtml';
|
export * from './SanitizeHtml';
|
||||||
export * from './SetLocalStorage';
|
export * from './SetLocalStorage';
|
||||||
|
|||||||
@@ -0,0 +1,43 @@
|
|||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import { isSafeExternalUrl } from './isSafeExternalUrl';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Guard for URLs opened from user-controlled content (chat links, external
|
||||||
|
* image/photo links). Only plain web URLs may be opened — never script- or
|
||||||
|
* data-bearing schemes that run in the opener's origin.
|
||||||
|
*/
|
||||||
|
describe('isSafeExternalUrl', () =>
|
||||||
|
{
|
||||||
|
it('accepts http and https URLs', () =>
|
||||||
|
{
|
||||||
|
expect(isSafeExternalUrl('http://example.com/path')).toBe(true);
|
||||||
|
expect(isSafeExternalUrl('https://example.com/path?q=1#x')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects javascript: URLs', () =>
|
||||||
|
{
|
||||||
|
expect(isSafeExternalUrl('javascript:alert(1)')).toBe(false);
|
||||||
|
expect(isSafeExternalUrl('JavaScript:alert(1)')).toBe(false);
|
||||||
|
expect(isSafeExternalUrl(' javascript:alert(1)')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects data: and vbscript: URLs', () =>
|
||||||
|
{
|
||||||
|
expect(isSafeExternalUrl('data:text/html,<script>alert(1)</script>')).toBe(false);
|
||||||
|
expect(isSafeExternalUrl('vbscript:msgbox(1)')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects file: and other non-web schemes', () =>
|
||||||
|
{
|
||||||
|
expect(isSafeExternalUrl('file:///etc/passwd')).toBe(false);
|
||||||
|
expect(isSafeExternalUrl('about:blank')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects empty / malformed input', () =>
|
||||||
|
{
|
||||||
|
expect(isSafeExternalUrl('')).toBe(false);
|
||||||
|
expect(isSafeExternalUrl(null as unknown as string)).toBe(false);
|
||||||
|
expect(isSafeExternalUrl('not a url')).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,21 @@
|
|||||||
|
/**
|
||||||
|
* Returns true only for plain web URLs (http/https). Used to gate URLs that
|
||||||
|
* originate from user-controlled content before they are opened — never let a
|
||||||
|
* `javascript:`, `data:`, `vbscript:`, `file:` … scheme reach `window.open`,
|
||||||
|
* which would run in the opener's origin.
|
||||||
|
*/
|
||||||
|
export const isSafeExternalUrl = (url: string): boolean =>
|
||||||
|
{
|
||||||
|
if(!url || (typeof url !== 'string')) return false;
|
||||||
|
|
||||||
|
try
|
||||||
|
{
|
||||||
|
const protocol = new URL(url.trim()).protocol;
|
||||||
|
|
||||||
|
return ((protocol === 'http:') || (protocol === 'https:'));
|
||||||
|
}
|
||||||
|
catch
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
};
|
||||||
@@ -321,7 +321,7 @@ export const GuideToolView: FC<{}> = props =>
|
|||||||
return;
|
return;
|
||||||
case 'forum_link':
|
case 'forum_link':
|
||||||
const url: string = GetConfigurationValue<string>('group.homepage.url', '').replace('%groupid%', GetConfigurationValue<string>('guide.help.alpha.groupid', '0'));
|
const url: string = GetConfigurationValue<string>('group.homepage.url', '').replace('%groupid%', GetConfigurationValue<string>('guide.help.alpha.groupid', '0'));
|
||||||
window.open(url);
|
window.open(url, '_blank', 'noopener,noreferrer');
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}, [ isHandlingBullyReports, isHandlingGuideRequests, isHandlingHelpRequests, simpleAlert ]);
|
}, [ isHandlingBullyReports, isHandlingGuideRequests, isHandlingHelpRequests, simpleAlert ]);
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { FC } from 'react';
|
import { FC } from 'react';
|
||||||
import { GetSessionDataManager } from '@nitrots/nitro-renderer';
|
import { GetSessionDataManager } from '@nitrots/nitro-renderer';
|
||||||
import { GetConfigurationValue, LocalizeText, ReportType } from '../../../../api';
|
import { GetConfigurationValue, isSafeExternalUrl, LocalizeText, ReportType } from '../../../../api';
|
||||||
import { NitroCardContentView, NitroCardHeaderView, NitroCardView } from '../../../../common';
|
import { NitroCardContentView, NitroCardHeaderView, NitroCardView } from '../../../../common';
|
||||||
import { useFurnitureExternalImageWidget, useHelp } from '../../../../hooks';
|
import { useFurnitureExternalImageWidget, useHelp } from '../../../../hooks';
|
||||||
import { CameraWidgetShowPhotoView } from '../../../camera/views/CameraWidgetShowPhotoView';
|
import { CameraWidgetShowPhotoView } from '../../../camera/views/CameraWidgetShowPhotoView';
|
||||||
@@ -15,10 +15,9 @@ export const FurnitureExternalImageView: FC<{}> = props =>
|
|||||||
const handleOpenFullPhoto = () =>
|
const handleOpenFullPhoto = () =>
|
||||||
{
|
{
|
||||||
const photoUrl = currentPhotos[currentPhotoIndex].w.replace('_small.png', '.png');
|
const photoUrl = currentPhotos[currentPhotoIndex].w.replace('_small.png', '.png');
|
||||||
if (photoUrl)
|
if (photoUrl && isSafeExternalUrl(photoUrl))
|
||||||
{
|
{
|
||||||
console.log('Opened photo URL:', photoUrl);
|
window.open(photoUrl, '_blank', 'noopener,noreferrer');
|
||||||
window.open(photoUrl, '_blank');
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -639,7 +639,7 @@ export const YouTubePlayerView: FC<{}> = () =>
|
|||||||
const url = `https://twitter.com/intent/tweet?text=${encodeURIComponent(
|
const url = `https://twitter.com/intent/tweet?text=${encodeURIComponent(
|
||||||
'Now watching: https://youtube.com/watch?v=${videoId}',
|
'Now watching: https://youtube.com/watch?v=${videoId}',
|
||||||
)}`;
|
)}`;
|
||||||
window.open(url, '_blank');
|
window.open(url, '_blank', 'noopener,noreferrer');
|
||||||
}
|
}
|
||||||
}}
|
}}
|
||||||
disabled={!videoId}
|
disabled={!videoId}
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { useBetween } from 'use-between';
|
import { useBetween } from 'use-between';
|
||||||
import { LocalizeText } from '../api';
|
import { isSafeExternalUrl, LocalizeText } from '../api';
|
||||||
import { useNotification } from './notification';
|
import { useNotification } from './notification';
|
||||||
|
|
||||||
const useOnClickChatState = () =>
|
const useOnClickChatState = () =>
|
||||||
@@ -15,9 +15,13 @@ const useOnClickChatState = () =>
|
|||||||
|
|
||||||
const url = event.target.href;
|
const url = event.target.href;
|
||||||
|
|
||||||
|
// Never open a URL that came from chat unless it is a plain web link —
|
||||||
|
// a javascript:/data: href would otherwise run in our origin.
|
||||||
|
if(!isSafeExternalUrl(url)) return;
|
||||||
|
|
||||||
showConfirm(LocalizeText('chat.confirm.openurl', [ 'url' ], [ url ]), () =>
|
showConfirm(LocalizeText('chat.confirm.openurl', [ 'url' ], [ url ]), () =>
|
||||||
{
|
{
|
||||||
window.open(url, '_blank');
|
window.open(url, '_blank', 'noopener,noreferrer');
|
||||||
}, null, null, null, LocalizeText('generic.alert.title'), null);
|
}, null, null, null, LocalizeText('generic.alert.title'), null);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user