Skip to content

Commit

Permalink
Merge pull request from GHSA-jwv5-8mqv-g387
Browse files Browse the repository at this point in the history
* 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 <alexandre_gaudreault@intuit.com>

* linting

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

---------

Signed-off-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
  • Loading branch information
Ry0taK and agaudreault committed Mar 13, 2024
1 parent 51cfd50 commit 479b554
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 13 deletions.
Expand Up @@ -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();
Expand Down Expand Up @@ -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: (
<React.Fragment>
{urls
.map(item => item.split('|'))
.map((parts, i) => (
<a key={i} href={parts.length > 1 ? parts[1] : parts[0]} target='__blank'>
{parts[0]} &nbsp;
{urls.map((url, i) => {
return (
<a key={i} href={url.ref} target='__blank'>
{url.title} &nbsp;
</a>
))}
);
})}
</React.Fragment>
)
});
Expand Down
68 changes: 67 additions & 1 deletion 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(() => {
Expand All @@ -7,6 +7,16 @@ test('rejects malicious URLs', () => {
expect(() => {
const _ = new ExternalLink('data:text/html;<h1>hi</h1>');
}).toThrowError(InvalidExternalLinkError);
expect(() => {
const _ = new ExternalLink('title|data:text/html;<h1>hi</h1>');
}).toThrowError(InvalidExternalLinkError);
expect(() => {
const _ = new ExternalLink('data:title|data:text/html;<h1>hi</h1>');
}).toThrowError(InvalidExternalLinkError);

expect(() => {
const _ = new ExternalLink('data:title|https://localhost:8080/applications');
}).not.toThrowError(InvalidExternalLinkError);
});

test('allows absolute URLs', () => {
Expand All @@ -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')
});
18 changes: 14 additions & 4 deletions ui/src/app/applications/components/application-urls.tsx
Expand Up @@ -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 {
Expand All @@ -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 && (
<div className='applications-list__external-links-icon-container'>
Expand Down

0 comments on commit 479b554

Please sign in to comment.