Skip to content

Commit

Permalink
add encodeURIComponent for WebUI data URLs in FE code (#27636)
Browse files Browse the repository at this point in the history
* add encodeURIComponent for WebUI data URLs in brave_wallet_ui components

It seems we shouldn't be updating chrome://favicon or brave://wallet URIs, so
this is an updated commit that addresses just the limited set of URIs that
require encodedURIComponents usage.

* Mostly working. Need to fix image rendering from untrusted frames

* Fix Market and NFT Icons

---------

Co-authored-by: Douglas Daniel <[email protected]>
  • Loading branch information
kdenhartog and Douglashdaniel authored Feb 27, 2025
1 parent f504bae commit 0e23cf8
Show file tree
Hide file tree
Showing 21 changed files with 84 additions and 53 deletions.
9 changes: 8 additions & 1 deletion browser/ui/webui/brave_wallet/market/market_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

#include <string>

#include "brave/browser/ui/webui/untrusted_sanitized_image_source.h"
#include "brave/components/brave_wallet/browser/brave_wallet_constants.h"
#include "brave/components/constants/webui_url_constants.h"
#include "brave/components/l10n/common/localization_util.h"
#include "brave/components/market_display/resources/grit/market_display_generated_map.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/grit/browser_resources.h"
#include "chrome/grit/generated_resources.h"
#include "components/grit/brave_components_resources.h"
#include "content/public/browser/url_data_source.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui_data_source.h"
#include "ui/webui/resources/grit/webui_resources.h"
Expand Down Expand Up @@ -55,7 +58,7 @@ UntrustedMarketUI::UntrustedMarketUI(content::WebUI* web_ui)
untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ImgSrc,
std::string("img-src 'self' https://assets.cgproxy.brave.com "
"chrome-untrusted://resources;"));
"chrome-untrusted://resources chrome-untrusted://image;"));

untrusted_source->AddResourcePath("load_time_data_deprecated.js",
IDR_WEBUI_JS_LOAD_TIME_DATA_DEPRECATED_JS);
Expand All @@ -67,6 +70,10 @@ UntrustedMarketUI::UntrustedMarketUI(content::WebUI* web_ui)
untrusted_source->AddString("braveWalletNftBridgeUrl", kUntrustedNftURL);
untrusted_source->AddString("braveWalletMarketUiBridgeUrl",
kUntrustedMarketURL);

Profile* profile = Profile::FromWebUI(web_ui);
content::URLDataSource::Add(
profile, std::make_unique<UntrustedSanitizedImageSource>(profile));
}

UntrustedMarketUI::~UntrustedMarketUI() = default;
Expand Down
10 changes: 9 additions & 1 deletion browser/ui/webui/brave_wallet/nft/nft_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

#include <string>

#include "brave/browser/ui/webui/untrusted_sanitized_image_source.h"
#include "brave/components/brave_wallet/browser/brave_wallet_constants.h"
#include "brave/components/constants/webui_url_constants.h"
#include "brave/components/l10n/common/localization_util.h"
#include "brave/components/nft_display/resources/grit/nft_display_generated_map.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/grit/browser_resources.h"
#include "chrome/grit/generated_resources.h"
#include "components/grit/brave_components_resources.h"
#include "content/public/browser/url_data_source.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui_data_source.h"
#include "ui/webui/resources/grit/webui_resources.h"
Expand Down Expand Up @@ -59,7 +62,12 @@ UntrustedNftUI::UntrustedNftUI(content::WebUI* web_ui)
kUntrustedMarketURL);
untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ImgSrc,
std::string("img-src 'self' https: data:;"));
std::string("img-src 'self' chrome-untrusted://resources "
"chrome-untrusted://image;"));

Profile* profile = Profile::FromWebUI(web_ui);
content::URLDataSource::Add(
profile, std::make_unique<UntrustedSanitizedImageSource>(profile));
}

UntrustedNftUI::~UntrustedNftUI() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ export class BaseQueryCache {
}
return token.logo.startsWith('ipfs://')
? (await this.getIpfsGatewayTranslatedNftUrl(token.logo)) || ''
: `chrome://erc-token-images/${token.logo}`
: token.logo
}

getNftMetadata = async (tokenArg: GetBlockchainTokenIdArg) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ export const tokenSuggestionsEndpoints = ({
if (logo !== '' && !isRemoteImageURL(logo)) {
try {
// attempt property reassignment
request.token.logo = `chrome://erc-token-images/${logo}`
request.token.logo = `chrome://image/?url=${encodeURIComponent(logo)}&staticEncode=true`
} catch {
request.token = {
...request.token,
logo: `chrome://erc-token-images/${logo}`
logo: `chrome://image/?url=${encodeURIComponent(logo)}&staticEncode=true`
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export const AssetNameAndIcon = (props: Props) => {
return (
<StyledWrapper>
<AssetIcon
src={assetLogo}
src={`chrome-untrusted://image?url=${encodeURIComponent(
assetLogo
)}&staticEncode=true`}
loading='lazy'
/>
<NameAndSymbolWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const AccountTypeItem = (props: Props) => {
return (
<StyledWrapper>
<LeftSide>
<NetworkIcon src={icon} />
<NetworkIcon src={`chrome://image?url=${encodeURIComponent(icon)}&staticEncode=true`} />
<InfoColumn>
<Title>{title}</Title>
<Description>{description}</Description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const DappListItem = React.forwardRef<
<Column>
{isHttpsUrl(dapp.logo) ? (
<img
src={`chrome://image?${dapp.logo}`}
src={`chrome://image?url=${encodeURIComponent(dapp.logo)}&staticEncode=true`}
height={40}
width={40}
alt={dapp.name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const DappDetails = ({ dapp, ...rest }: DappDetailsProps) => {
>
{isHttpsUrl(dapp.logo) ? (
<img
src={`chrome://image?${dapp.logo}`}
src={`chrome://image?url=${encodeURIComponent(dapp.logo)}&staticEncode=true`}
width={72}
height={72}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const AuthorizeHardwareDeviceIFrame = (props: Props) => {
const src =
LEDGER_BRIDGE_URL +
`?targetUrl=${encodeURIComponent(window.origin)}` +
`&bridgeType=${bridgeTypeFromCoin(props.coinType)}`
`&bridgeType=${encodeURIComponent(bridgeTypeFromCoin(props.coinType))}`
return (
<StyledIFrame
src={src}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const BuyOptionItem = (props: Props) => {
return (
<StyledWrapper layoutType={layoutType}>
<Row justifyContent='flex-start'>
<Logo src={icon} />
<Logo src={`chrome://image?url=${encodeURIComponent(icon)}&staticEncode=true`} />
<Content>
<Name>{name}</Name>
<Description>{description}</Description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export const CreateNetworkIcon = ({ network, marginRight, size }: Props) => {

// complex compute + render
const networkIcon = network.iconUrls[0]
const isSandboxUrl = networkIcon?.startsWith('chrome://erc-token-images/')
const isSandboxUrl = networkIcon?.startsWith('chrome://image/')
const networkImageURL = isSandboxUrl
? stripERC20TokenImageURL(networkIcon)
: networkIcon
Expand Down Expand Up @@ -124,7 +124,11 @@ export const CreateNetworkIcon = ({ network, marginRight, size }: Props) => {
>
<NetworkIcon
size={size}
icon={isRemoteURL ? `chrome://image?${networkImageURL}` : networkIcon}
icon={
isRemoteURL
? `chrome://image?url=${encodeURIComponent(networkImageURL)}&staticEncode=true`
: networkIcon
}
/>
</IconWrapper>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ import { BraveWallet } from '../../../constants/types'

// Utils
import {
stripERC20TokenImageURL,
isRemoteImageURL,
isValidIconExtension,
isDataURL,
isComponentInStorybook,
stripChromeImageURL
isComponentInStorybook
} from '../../../utils/string-utils'
import { isNativeAsset } from '../../../utils/asset-utils'

Expand Down Expand Up @@ -74,34 +72,34 @@ export function withPlaceholderIcon<
const nativeAssetLogo =
isNative && asset ? makeNativeAssetLogo(asset.symbol, asset.chainId) : ''

const tokenImageURL = stripERC20TokenImageURL(
nativeAssetLogo || asset?.logo || ''
)
const isRemoteURL = isRemoteImageURL(tokenImageURL)
const initialTokenImageURL = nativeAssetLogo || asset?.logo || ''

const isRemoteURL = isRemoteImageURL(initialTokenImageURL)

const isNonFungibleToken = asset?.isNft || asset?.isErc721

const tokenImageURL =
isRemoteURL || isNonFungibleToken || isNative
? initialTokenImageURL
: `chrome://erc-token-images/${encodeURIComponent(
initialTokenImageURL
)}`

// memos + computed
const isValidIcon = React.useMemo(() => {
if (isStorybook) {
return !!asset?.logo
}

const isDataUri = isDataURL(asset?.logo)
const isDataUri = isDataURL(tokenImageURL)

if (isRemoteURL || isDataUri) {
return tokenImageURL?.includes('data:image/') ||
isNonFungibleToken
return tokenImageURL?.includes('data:image/') || isNonFungibleToken
? true
: isValidIconExtension(new URL(asset?.logo || '').pathname)
: isValidIconExtension(new URL(tokenImageURL).pathname)
}
return false
}, [
asset?.logo,
isRemoteURL,
tokenImageURL,
isNonFungibleToken
])
}, [asset?.logo, isRemoteURL, tokenImageURL, isNonFungibleToken])

const needsPlaceholder =
(tokenImageURL === '' || !isValidIcon) && nativeAssetLogo === ''
Expand All @@ -118,7 +116,9 @@ export function withPlaceholderIcon<

const remoteImage = React.useMemo(() => {
if (isRemoteURL) {
return isStorybook ? tokenImageURL || '' : `chrome://image?${tokenImageURL}`
return `chrome://image?url=${encodeURIComponent(
tokenImageURL
)}&staticEncode=true`
}
return ''
}, [isRemoteURL, tokenImageURL])
Expand All @@ -128,7 +128,7 @@ export function withPlaceholderIcon<
return null
}

const icon = nativeAssetLogo || (isRemoteURL ? remoteImage : asset?.logo)
const icon = nativeAssetLogo || (isRemoteURL ? remoteImage : tokenImageURL)

if (needsPlaceholder || !icon) {
return (
Expand Down Expand Up @@ -157,7 +157,7 @@ export function withPlaceholderIcon<
{...(wrappedComponentProps as PROPS_FOR_FUNCTION & {
icon?: undefined
})}
icon={isStorybook ? stripChromeImageURL(tokenImageURL) : icon}
icon={isStorybook ? tokenImageURL : icon}
/>
</IconWrapper>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import { skipToken } from '@reduxjs/toolkit/query'
import {
isComponentInStorybook,
isIpfs,
stripERC20TokenImageURL
stripERC20TokenImageURL,
stripChromeImageURL
} from '../../../utils/string-utils'

// hooks
Expand Down Expand Up @@ -54,7 +55,9 @@ export const NftIcon = (props: NftIconProps) => {
const nftIframeUrl = React.useMemo(() => {
const urlParams = new URLSearchParams({
displayMode: 'icon',
icon: encodeURI(remoteImage || '')
icon: `chrome-untrusted://image?url=${encodeURIComponent(
remoteImage
)}&staticEncode=true`
})
return `chrome-untrusted://nft-display?${urlParams.toString()}`
}, [remoteImage])
Expand Down Expand Up @@ -97,7 +100,7 @@ export const NftIcon = (props: NftIconProps) => {

export const StorybookNftIcon = (props: NftIconProps) => {
const { icon, responsive, iconStyles } = props
const tokenImageURL = icon ? stripERC20TokenImageURL(icon) : ''
const tokenImageURL = icon ? stripChromeImageURL(icon) : ''

return responsive ? (
<NftImageResponsiveIframe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,7 @@ export const NftScreen = ({ selectedAsset, tokenNetwork }: Props) => {
spamTokenBalancesRegistry
) && (
<>
<Alert
type='info'
>
{getLocale('braveWalletUnownedNftAlert')}
</Alert>
<Alert type='info'>{getLocale('braveWalletUnownedNftAlert')}</Alert>
<VerticalSpace space='24px' />
</>
)}
Expand All @@ -249,7 +245,9 @@ export const NftScreen = ({ selectedAsset, tokenNetwork }: Props) => {
<NftMultimedia
as='img'
visible={!isFetchingNFTMetadata}
src={nftMetadata?.imageURL}
src={`chrome-untrusted://image?url=${encodeURIComponent(
nftMetadata?.imageURL || ''
)}&staticEncode=true`}
/>
) : (
<NftMultimedia
Expand Down
5 changes: 4 additions & 1 deletion components/brave_wallet_ui/nft/nft_script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ const imageSrc = stripChromeImageURL(imageUrl) ?? Placeholder
function render() {
const imgEl = document.getElementById('nft-image') as HTMLImageElement
// update img src
imgEl.src = displayMode === 'details' && nftMetadata ? mediaUrl : imageSrc
imgEl.src =
displayMode === 'details' && nftMetadata
? `chrome-untrusted://image?url=${encodeURIComponent(mediaUrl)}`
: imageSrc
}

document.addEventListener('DOMContentLoaded', render)
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const BuyQuote = ({
alignItems='center'
>
{providerImageUrl ? (
<ProviderImage src={`chrome://image?${providerImageUrl}`} />
<ProviderImage src={`chrome://image?url=${encodeURIComponent(providerImageUrl)}&staticEncode=true`} />
) : null}

<Column alignItems='flex-start'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export const AssetListItem = ({
gap='16px'
>
<IconsWrapper>
<AssetImage src={`chrome://image?${symbolImageUrl}`} />
<AssetImage src={`chrome://image?url=${encodeURIComponent(symbolImageUrl ?? '')}&staticEncode=true`} />
<NetworkIconWrapper>
<CreateNetworkIcon
network={network}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ export const SelectAssetButton = (props: SelectAssetButtonProps) => {
<IconsWrapper>
<AssetIcon
size='40px'
src={`chrome://image?${selectedAsset?.symbolImageUrl}`}
src={`chrome://image?url=${
selectedAsset?.symbolImageUrl
}&staticEncode=true`}
/>
{tokensNetwork && (
<NetworkIconWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ export const CurrencyListItem = ({
justifyContent='flex-start'
gap='16px'
>
<CurrencyImage src={`chrome://image?${currency.symbolImageUrl}`} />
<CurrencyImage
src={`chrome://image?url=${encodeURIComponent(
currency.symbolImageUrl ?? ''
)}&staticEncode=true`}
/>
<CurrencyName>{currency.name}</CurrencyName>
</Row>
<Row
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,7 @@ export const FundWalletScreen = ({ isAndroid }: Props) => {
</Dropdown>
<Dropdown
value={selectedPaymentMethod.paymentMethod}
onChange={(detail) =>
onSelectPaymentMethod(detail.value!)
}
onChange={(detail) => onSelectPaymentMethod(detail.value!)}
disabled={
isFetchingFirstTimeQuotes || isLoadingPaymentMethods
}
Expand All @@ -246,8 +244,10 @@ export const FundWalletScreen = ({ isAndroid }: Props) => {
<PaymentMethodIcon
src={
isStorybook
? logoUrl
: `chrome://image?${logoUrl}`
? logoUrl ?? ''
: `chrome://image?url=${encodeURIComponent(
logoUrl ?? ''
)}&staticEncode=true`
}
/>
{paymentMethod.name}
Expand Down
Loading

0 comments on commit 0e23cf8

Please sign in to comment.