Wallet Clipboard Security (#30317)

Co-authored-by: Douglas Daniel <douglashdaniel@gmail.com>
This commit is contained in:
Christian Mazakas
2025-08-04 07:17:23 -07:00
committed by GitHub
co-authored by Douglas Daniel
parent 59661f743b
commit 64124b0733
13 changed files with 137 additions and 20 deletions
+2 -1
View File
@@ -343,7 +343,8 @@ static_library("browser") {
"//third_party/boringssl",
"//third_party/re2",
"//tools/json_schema_compiler:generated_api_util",
"//ui/base:base",
"//ui/base",
"//ui/base/clipboard",
"//url",
]
@@ -17,6 +17,7 @@
#include "base/logging.h"
#include "base/notreached.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "brave/components/brave_wallet/browser/account_discovery_manager.h"
#include "brave/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h"
@@ -44,6 +45,7 @@
#include "components/prefs/scoped_user_pref_update.h"
#include "components/regional_capabilities/regional_capabilities_prefs.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/origin.h"
@@ -1959,6 +1961,26 @@ void BraveWalletService::SetTransactionSimulationOptInStatus(
::brave_wallet::SetTransactionSimulationOptInStatus(profile_prefs_, status);
}
void BraveWalletService::WriteToClipboard(const std::string& text,
bool is_sensitive) {
// We manually disable the iOS builds here because of an upstream bug in how
// Chromium is adding sources to the clipboard component. It only
// conditionally adds the iOS sources when use_blink=true, which unfortunately
// leads to a whole slew of unresolved symbols during linking.
// https://source.chromium.org/chromium/chromium/src/+/066b9c51bfb0a1eddcfefa7aa809348ea181f8ac:ui/base/clipboard/BUILD.gn;l=21-27
#if !BUILDFLAG(IS_IOS)
ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste);
std::u16string out;
base::UTF8ToUTF16(text.data(), text.size(), &out);
scw.WriteText(out);
if (is_sensitive) {
scw.MarkAsConfidential();
}
#else
NOTREACHED();
#endif
}
base::CallbackListSubscription
BraveWalletService::RegisterSignMessageRequestAddedCallback(
base::RepeatingClosure cb) {
@@ -269,6 +269,8 @@ class BraveWalletService : public KeyedService,
void SetTransactionSimulationOptInStatus(
mojom::BlowfishOptInStatus status) override;
void WriteToClipboard(const std::string& text, bool is_sensitive) override;
// BraveWalletServiceDelegate::Observer:
void OnActiveOriginChanged(const mojom::OriginInfoPtr& origin_info) override;
@@ -1328,7 +1328,7 @@ interface MeldIntegrationService {
// destination_currency_code, country, source_amount, source_currency_code
GetCryptoQuotes(string country, string source_currency_code,
string destination_currency_code, double source_amount, string? account,
string? payment_method)
string? payment_method)
=> (array<MeldCryptoQuote>? crypto_quotes, array<string>? error);
// Obtains the list of payment methods
@@ -2773,6 +2773,8 @@ interface BraveWalletService {
GetTransactionSimulationOptInStatus() => (BlowfishOptInStatus status);
SetTransactionSimulationOptInStatus(BlowfishOptInStatus status);
WriteToClipboard(string text, bool is_sensitive);
};
// For reporting wallet related P3A metrics.
@@ -513,6 +513,12 @@ export class MockedWalletApiProxy {
}
return { network: mockEthMainnet }
},
writeToClipboard: async (text: string, isConfidential: boolean) => {
return {
data: true,
}
},
}
swapService: Partial<InstanceType<typeof BraveWallet.SwapServiceInterface>> =
@@ -8,14 +8,40 @@ import {
useTemporaryCopyToClipboard,
} from './use-copy-to-clipboard'
// Utils
import {
createMockStore,
renderHookOptionsWithMockStore,
} from '../../utils/test-utils'
describe('useCopyToClipboard Hook', () => {
it('should have false as initial state', () => {
const { result } = renderHook(() => useCopyToClipboard())
const store = createMockStore({})
const renderOptions = renderHookOptionsWithMockStore(store)
const { result } = renderHook(() => useCopyToClipboard(), renderOptions)
expect(result.current.isCopied).toBe(false)
})
it('should change copied to true when copyText is called', async () => {
const { result } = renderHook(() => useCopyToClipboard())
const store = createMockStore({})
const renderOptions = renderHookOptionsWithMockStore(store)
const { result } = renderHook(() => useCopyToClipboard(), renderOptions)
await act(async () => {
await result.current.copyToClipboard('some text')
})
expect(result.current.isCopied).toBe(true)
})
it('should copy to clipboard confidentially', async () => {
const store = createMockStore({})
const renderOptions = renderHookOptionsWithMockStore(store)
const { result } = renderHook(
() => useCopyToClipboard(undefined, true),
renderOptions,
)
await act(async () => {
await result.current.copyToClipboard('some text')
})
@@ -29,9 +55,12 @@ describe('useTemporaryCopyToClipboard Hook', () => {
jest.clearAllTimers()
const timeoutTime = 5000 // 5 seconds
const store = createMockStore({})
const renderOptions = renderHookOptionsWithMockStore(store)
const { result } = renderHook(() =>
useTemporaryCopyToClipboard(timeoutTime),
const { result } = renderHook(
() => useTemporaryCopyToClipboard(timeoutTime),
renderOptions,
)
await act(async () => {
@@ -7,21 +7,36 @@ import * as React from 'react'
// utils
import { copyToClipboard } from '../../utils/copy-to-clipboard'
import {
useCopyToClipboardConfidentiallyMutation, //
} from '../slices/api.slice'
const temporaryCopyTimeout = 5000 // 5s
const copiedMessageTimeout = 1500 // 1.5s
export const useTemporaryCopyToClipboard = (
timeoutMs: number = temporaryCopyTimeout,
isConfidential = false,
) => {
// mutations
const [copyToClipboardConfidentially] =
useCopyToClipboardConfidentiallyMutation()
// state
const [isCopied, setIsCopied] = React.useState(false)
// methods
const temporaryCopyToClipboard = React.useCallback(async (value: string) => {
await copyToClipboard(value)
setIsCopied(true)
}, [])
const temporaryCopyToClipboard = React.useCallback(
async (value: string) => {
if (isConfidential) {
await copyToClipboardConfidentially({ text: value })
} else {
await copyToClipboard(value)
}
setIsCopied(true)
},
[isConfidential, copyToClipboardConfidentially],
)
// effects
React.useEffect(() => {
@@ -32,7 +47,11 @@ export const useTemporaryCopyToClipboard = (
// clear the clipboard after a set time
const timer = window.setTimeout(async () => {
await copyToClipboard('')
if (isConfidential) {
await copyToClipboardConfidentially({ text: '' })
} else {
await copyToClipboard('')
}
setIsCopied(false)
}, timeoutMs)
@@ -40,7 +59,7 @@ export const useTemporaryCopyToClipboard = (
return () => {
timer && clearTimeout(timer)
}
}, [isCopied, timeoutMs])
}, [isCopied, timeoutMs, isConfidential, copyToClipboardConfidentially])
return {
temporaryCopyToClipboard,
@@ -48,15 +67,29 @@ export const useTemporaryCopyToClipboard = (
}
}
export const useCopyToClipboard = (timeoutMs = copiedMessageTimeout) => {
export const useCopyToClipboard = (
timeoutMs = copiedMessageTimeout,
isConfidential = false,
) => {
// mutations
const [copyToClipboardConfidentially] =
useCopyToClipboardConfidentiallyMutation()
// state
const [isCopied, setIsCopied] = React.useState(false)
// methods
const _copyToClipboard = React.useCallback(async (value: string) => {
await copyToClipboard(value)
setIsCopied(true)
}, [])
const _copyToClipboard = React.useCallback(
async (value: string) => {
if (isConfidential) {
await copyToClipboardConfidentially({ text: value })
} else {
await copyToClipboard(value)
}
setIsCopied(true)
},
[isConfidential, copyToClipboardConfidentially],
)
const resetCopyState = React.useCallback(() => {
setIsCopied(false)
@@ -312,6 +312,7 @@ export const {
useLazyGetTransactionsQuery,
useLazyGetUserTokensRegistryQuery,
useLazyGetZCashAccountInfoQuery,
useCopyToClipboardConfidentiallyMutation,
useLockWalletMutation,
useMakeAccountShieldedMutation,
useNewUnapprovedTxAddedMutation,
@@ -512,6 +512,16 @@ export const walletEndpoints = ({
}
},
}),
copyToClipboardConfidentially: mutation<boolean, { text: string }>({
queryFn: async ({ text }, { endpoint }, extraOptions, baseQuery) => {
const { data: api } = baseQuery(undefined)
api.braveWalletService.writeToClipboard(text, true)
return {
data: true,
}
},
}),
}
}
@@ -427,7 +427,10 @@ export const AccountSettingsModal = () => {
=== BraveWallet.CoinType.FIL && (
<Alert type='warning'>{filPrivateKeyFormatDescription}</Alert>
)}
<CopyTooltip text={privateKey}>
<CopyTooltip
text={privateKey}
isConfidential={true}
>
<PrivateKeyBubble>{privateKey}</PrivateKeyBubble>
</CopyTooltip>
</>
@@ -22,6 +22,7 @@ type Props = {
tooltipText?: string
actionText?: string
text?: string
isConfidential?: boolean
} & ToolTipProps
export const CopyTooltip = ({
@@ -29,9 +30,10 @@ export const CopyTooltip = ({
tooltipText,
actionText,
text,
isConfidential,
...tipProps
}: Props) => {
const { isCopied, copyToClipboard } = useCopyToClipboard(1500)
const { isCopied, copyToClipboard } = useCopyToClipboard(1500, isConfidential)
const handleClick = React.useCallback(async () => {
if (text) {
@@ -71,7 +71,10 @@ export const BackupRecoveryPhrase = () => {
const [report] = useReportOnboardingActionMutation()
// custom hooks
const { isCopied, temporaryCopyToClipboard } = useTemporaryCopyToClipboard()
const { isCopied, temporaryCopyToClipboard } = useTemporaryCopyToClipboard(
undefined,
true,
)
// methods
const skipBackup = () => {
@@ -401,6 +401,9 @@ class MockBraveWalletService: BraveWalletBraveWalletService {
func discoverAssetsOnAllSupportedChains(bypassRateLimit: Bool) {
}
func writeToClipboard(text: String, isSensitive: Bool) {
}
func transactionSimulationOptInStatus(
completion: @escaping (BraveWallet.BlowfishOptInStatus) -> Void
) {