Skip to content

Commit 4b673b9

Browse files
authored
Merge pull request #585 from kleros/fix/improve-xss-security
fix: improve security
2 parents 064af54 + 8206be9 commit 4b673b9

3 files changed

Lines changed: 50 additions & 17 deletions

File tree

src/components/attachment.js

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from "react";
22
import t from "prop-types";
33
import styled from "styled-components/macro";
4-
import { Divider, Popover } from "antd";
4+
import { Divider, Popover, Tooltip } from "antd";
55
import isImage from "is-image";
66
import isTextPath from "is-text-path";
77
import isVideo from "is-video";
@@ -10,6 +10,7 @@ import { ReactComponent as Image } from "../assets/images/image.svg";
1010
import { ReactComponent as Link } from "../assets/images/link.svg";
1111
import { ReactComponent as PDF } from "../assets/images/pdf.svg";
1212
import { ReactComponent as Video } from "../assets/images/video.svg";
13+
import { isSafeNavigationUrl } from "../utils/urlValidation";
1314

1415
const StyledPopover = styled(({ className, ...rest }) => (
1516
<Popover className={className} overlayClassName={className} {...rest} />
@@ -29,6 +30,11 @@ const StyledIFrame = styled.iframe`
2930
margin-top: -8px;
3031
width: 300px;
3132
`;
33+
const DisabledAttachment = styled.span`
34+
cursor: not-allowed;
35+
display: inline-flex;
36+
opacity: 0.5;
37+
`;
3238

3339
const isPDF = (extension) => extension.toLowerCase() === ".pdf";
3440

@@ -44,15 +50,31 @@ const Attachment = ({ URI, description, extension: _extension, previewURI, title
4450
else if (isVideo(extension)) Component = Video;
4551
else Component = Link;
4652
Component = <Component className="primary-purple-fill theme-fill" />;
53+
54+
const href = URI.replace(/^\/ipfs\//, "https://cdn.kleros.link/ipfs/");
55+
56+
// Unsafe attachment URL still indicate that a file was attached,
57+
// but render it as a disabled, non-clickable icon with an explanation.
58+
if (!isSafeNavigationUrl(href)) {
59+
return (
60+
<Tooltip
61+
overlayStyle={{ wordBreak: "break-all" }}
62+
title={`This attachment link was flagged as unsafe and has been disabled: "${href}"`}
63+
>
64+
<DisabledAttachment>{Component}</DisabledAttachment>
65+
</Tooltip>
66+
);
67+
}
68+
69+
const LinkedComponent = (
70+
<a href={href} rel="noopener noreferrer" target="_blank">
71+
{Component}
72+
</a>
73+
);
74+
4775
// No popover
4876
if (!title && !description) {
49-
if (URI)
50-
return (
51-
<a href={URI.replace(/^\/ipfs\//, "https://cdn.kleros.link/ipfs/")} rel="noopener noreferrer" target="_blank">
52-
{Component}
53-
</a>
54-
);
55-
return Component;
77+
return LinkedComponent;
5678
}
5779

5880
return (
@@ -72,13 +94,7 @@ const Attachment = ({ URI, description, extension: _extension, previewURI, title
7294
}
7395
title={title}
7496
>
75-
{URI ? (
76-
<a href={URI.replace(/^\/ipfs\//, "https://cdn.kleros.link/ipfs/")} rel="noopener noreferrer" target="_blank">
77-
{Component}
78-
</a>
79-
) : (
80-
Component
81-
)}
97+
{LinkedComponent}
8298
</StyledPopover>
8399
);
84100
};

src/components/case-details-card.jsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { getReadOnlyRpcUrl } from "../bootstrap/web3";
2929
import useGetDraws from "../hooks/use-get-draws";
3030
import arbitrableWhitelist from "../temp/arbitrable-whitelist";
3131
import { getAnswerString, RTA_LABEL } from "../temp/answer-string";
32+
import { isSafeNavigationUrl } from "../utils/urlValidation";
3233

3334
const { useDrizzle, useDrizzleState } = drizzleReactHooks;
3435
const { toBN, soliditySha3 } = Web3.utils;
@@ -504,6 +505,7 @@ export default function CaseDetailsCard({ ID }) {
504505
const { _v = "0" } = metaEvidence;
505506

506507
let url = normalizeIPFSUri(evidenceDisplayInterfaceURI);
508+
if (!isSafeNavigationUrl(url)) return null;
507509

508510
const injectedParams = {
509511
disputeID: ID,
@@ -686,8 +688,10 @@ export default function CaseDetailsCard({ ID }) {
686688
<Row>
687689
<Col span={24}>
688690
<StyledInnerCard actions={metaEvidenceActions}>
691+
{/* ReactMarkdown 4 escapes HTML by default. Changing this or bumping the major
692+
version without sanitizing the output could expose us to XSS attacks. */}
689693
<ReactMarkdown source={metaEvidence.description} />
690-
{metaEvidence.evidenceDisplayInterfaceURI && (
694+
{evidenceDisplayInterfaceURL && (
691695
<StyledIframeContainer>
692696
<iframe
693697
sandbox={
@@ -701,7 +705,7 @@ export default function CaseDetailsCard({ ID }) {
701705
/>
702706
</StyledIframeContainer>
703707
)}
704-
{metaEvidence.arbitrableInterfaceURI && (
708+
{metaEvidence.arbitrableInterfaceURI && isSafeNavigationUrl(metaEvidence.arbitrableInterfaceURI) && (
705709
<ArbitrableInterfaceDiv>
706710
<a href={metaEvidence.arbitrableInterfaceURI} target="_blank" rel="noopener noreferrer">
707711
<Icon type="double-right" style={{ marginRight: "5px" }} />

src/utils/urlValidation.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
function parseUrl(url) {
2+
if (!url || typeof url !== "string") return null;
3+
try {
4+
return new URL(url.trim());
5+
} catch {
6+
return null;
7+
}
8+
}
9+
10+
// Only https is allowed, rejects javascript:, data:, vbscript:, file:, etc.
11+
export function isSafeNavigationUrl(url) {
12+
return parseUrl(url)?.protocol === "https:";
13+
}

0 commit comments

Comments
 (0)