From 479b5544b57dc9ef767d49f7003f39602c480b71 Mon Sep 17 00:00:00 2001 From: RyotaK <49341894+Ry0taK@users.noreply.github.com> Date: Thu, 14 Mar 2024 03:26:47 +0900 Subject: [PATCH] Merge pull request from GHSA-jwv5-8mqv-g387 * fix: Validate external URLs for applicatins Signed-off-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com> * fix(ui): remove invalid external-link Signed-off-by: Alexandre Gaudreault * linting Signed-off-by: Alexandre Gaudreault --------- Signed-off-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com> Signed-off-by: Alexandre Gaudreault Co-authored-by: Alexandre Gaudreault --- .../application-summary.tsx | 16 ++--- .../components/application-urls.test.ts | 68 ++++++++++++++++++- .../components/application-urls.tsx | 18 +++-- 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/ui/src/app/applications/components/application-summary/application-summary.tsx b/ui/src/app/applications/components/application-summary/application-summary.tsx index 26773f2d3bc65..f38a380b50ea8 100644 --- a/ui/src/app/applications/components/application-summary/application-summary.tsx +++ b/ui/src/app/applications/components/application-summary/application-summary.tsx @@ -30,6 +30,7 @@ import {EditAnnotations} from './edit-annotations'; import './application-summary.scss'; import {DeepLinks} from '../../../shared/components/deep-links'; +import {ExternalLinks} from '../application-urls'; function swap(array: any[], a: number, b: number) { array = array.slice(); @@ -341,20 +342,19 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => { ) } ]; - - const urls = app.status.summary.externalURLs || []; + const urls = ExternalLinks(app.status.summary.externalURLs); if (urls.length > 0) { attributes.push({ title: 'URLs', view: ( - {urls - .map(item => item.split('|')) - .map((parts, i) => ( - 1 ? parts[1] : parts[0]} target='__blank'> - {parts[0]}   + {urls.map((url, i) => { + return ( + + {url.title}   - ))} + ); + })} ) }); diff --git a/ui/src/app/applications/components/application-urls.test.ts b/ui/src/app/applications/components/application-urls.test.ts index c9063561d01af..a3093a5a29c1d 100644 --- a/ui/src/app/applications/components/application-urls.test.ts +++ b/ui/src/app/applications/components/application-urls.test.ts @@ -1,4 +1,4 @@ -import {ExternalLink, InvalidExternalLinkError} from './application-urls'; +import { ExternalLink, ExternalLinks, InvalidExternalLinkError } from './application-urls'; test('rejects malicious URLs', () => { expect(() => { @@ -7,6 +7,16 @@ test('rejects malicious URLs', () => { expect(() => { const _ = new ExternalLink('data:text/html;

hi

'); }).toThrowError(InvalidExternalLinkError); + expect(() => { + const _ = new ExternalLink('title|data:text/html;

hi

'); + }).toThrowError(InvalidExternalLinkError); + expect(() => { + const _ = new ExternalLink('data:title|data:text/html;

hi

'); + }).toThrowError(InvalidExternalLinkError); + + expect(() => { + const _ = new ExternalLink('data:title|https://localhost:8080/applications'); + }).not.toThrowError(InvalidExternalLinkError); }); test('allows absolute URLs', () => { @@ -18,3 +28,59 @@ test('allows relative URLs', () => { window.location = new URL('https://localhost:8080/applications'); expect(new ExternalLink('/applications').ref).toEqual('/applications'); }); + + +test('URLs format', () => { + expect(new ExternalLink('https://localhost:8080/applications')).toEqual({ + ref: 'https://localhost:8080/applications', + title: 'https://localhost:8080/applications', + }) + expect(new ExternalLink('title|https://localhost:8080/applications')).toEqual({ + ref: 'https://localhost:8080/applications', + title: 'title', + }) +}); + + +test('malicious URLs from list to be removed', () => { + const urls: string[] = [ + 'javascript:alert("hi")', + 'https://localhost:8080/applications', + ] + const links = ExternalLinks(urls); + + expect(links).toHaveLength(1); + expect(links).toContainEqual({ + ref: 'https://localhost:8080/applications', + title: 'https://localhost:8080/applications', + }); +}); + + +test('list to be sorted', () => { + const urls: string[] = [ + 'https://a', + 'https://b', + 'a|https://c', + 'z|https://c', + 'x|https://d', + 'x|https://c', + ] + const links = ExternalLinks(urls); + + // 'a|https://c', + // 'x|https://c', + // 'x|https://d', + // 'z|https://c', + // 'https://a', + // 'https://b', + expect(links).toHaveLength(6); + expect(links[0].title).toEqual('a') + expect(links[1].title).toEqual('x') + expect(links[1].ref).toEqual('https://c') + expect(links[2].title).toEqual('x') + expect(links[2].ref).toEqual('https://d') + expect(links[3].title).toEqual('z') + expect(links[4].title).toEqual('https://a') + expect(links[5].title).toEqual('https://b') +}); diff --git a/ui/src/app/applications/components/application-urls.tsx b/ui/src/app/applications/components/application-urls.tsx index e6dc82458156d..4e4c6997ce386 100644 --- a/ui/src/app/applications/components/application-urls.tsx +++ b/ui/src/app/applications/components/application-urls.tsx @@ -29,7 +29,7 @@ export class ExternalLink { } } -export const ApplicationURLs = ({urls}: {urls: string[]}) => { +export const ExternalLinks = (urls?: string[]) => { const externalLinks: ExternalLink[] = []; for (const url of urls || []) { try { @@ -42,16 +42,26 @@ export const ApplicationURLs = ({urls}: {urls: string[]}) => { // sorted alphabetically & links with titles first externalLinks.sort((a, b) => { - if (a.title !== '' && b.title !== '') { + const hasTitle = (x: ExternalLink): boolean => { + return x.title !== x.ref && x.title !== ''; + }; + + if (hasTitle(a) && hasTitle(b) && a.title !== b.title) { return a.title > b.title ? 1 : -1; - } else if (a.title === '') { + } else if (hasTitle(b) && !hasTitle(a)) { return 1; - } else if (b.title === '') { + } else if (hasTitle(a) && !hasTitle(b)) { return -1; } return a.ref > b.ref ? 1 : -1; }); + return externalLinks; +}; + +export const ApplicationURLs = ({urls}: {urls: string[]}) => { + const externalLinks: ExternalLink[] = ExternalLinks(urls); + return ( ((externalLinks || []).length > 0 && (