Skip to content

Commit

Permalink
Revert "Fix UNSAFE_TODO for wallet (#26253)" (#26392)
Browse files Browse the repository at this point in the history
This reverts commit e840557.

The original PR introduces a specialisation to `base::internal` that
is not allowed, and would be Undefined Behaviour with `std::span`. This
has broken `cr132`.
  • Loading branch information
cdesouza-chromium authored Nov 6, 2024
1 parent 125c4f2 commit c728965
Show file tree
Hide file tree
Showing 74 changed files with 1,144 additions and 937 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ AndroidWalletPageUI::AndroidWalletPageUI(content::WebUI* web_ui,

// Add required resources.
if (url.host() == kWalletPageHost) {
webui::SetupWebUIDataSource(source, base::span(kBraveWalletPageGenerated),
IDR_WALLET_PAGE_HTML);
webui::SetupWebUIDataSource(
source,
UNSAFE_TODO(base::make_span(kBraveWalletPageGenerated,
kBraveWalletPageGeneratedSize)),
IDR_WALLET_PAGE_HTML);
} else {
NOTREACHED_IN_MIGRATION()
<< "Failed to find page resources for:" << url.path();
Expand Down
3 changes: 2 additions & 1 deletion browser/ui/webui/brave_wallet/ledger/ledger_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ UntrustedLedgerUI::UntrustedLedgerUI(content::WebUI* web_ui)
auto* untrusted_source = content::WebUIDataSource::CreateAndAdd(
web_ui->GetWebContents()->GetBrowserContext(), kUntrustedLedgerURL);
untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_LEDGER_BRIDGE_HTML);
untrusted_source->AddResourcePaths(base::span(kLedgerBridgeGenerated));
untrusted_source->AddResourcePaths(UNSAFE_TODO(
base::make_span(kLedgerBridgeGenerated, kLedgerBridgeGeneratedSize)));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
untrusted_source->OverrideContentSecurityPolicy(
Expand Down
12 changes: 10 additions & 2 deletions browser/ui/webui/brave_wallet/line_chart/line_chart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifdef UNSAFE_BUFFERS_BUILD
// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and
// convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif

#include "brave/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.h"

#include <string>
Expand Down Expand Up @@ -34,11 +40,13 @@ UntrustedLineChartUI::UntrustedLineChartUI(content::WebUI* web_ui)

untrusted_source->SetDefaultResource(
IDR_BRAVE_WALLET_LINE_CHART_DISPLAY_HTML);
untrusted_source->AddResourcePaths(base::span(kLineChartDisplayGenerated));
untrusted_source->AddResourcePaths(base::make_span(
kLineChartDisplayGenerated, kLineChartDisplayGeneratedSize));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
webui::SetupWebUIDataSource(untrusted_source,
base::span(kLineChartDisplayGenerated),
base::make_span(kLineChartDisplayGenerated,
kLineChartDisplayGeneratedSize),
IDR_BRAVE_WALLET_LINE_CHART_DISPLAY_HTML);
untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ScriptSrc,
Expand Down
16 changes: 12 additions & 4 deletions browser/ui/webui/brave_wallet/market/market_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifdef UNSAFE_BUFFERS_BUILD
// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and
// convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif

#include "brave/browser/ui/webui/brave_wallet/market/market_ui.h"

#include <string>
Expand Down Expand Up @@ -32,12 +38,14 @@ UntrustedMarketUI::UntrustedMarketUI(content::WebUI* web_ui)
untrusted_source->AddString(str.name, l10n_str);
}
untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML);
untrusted_source->AddResourcePaths(base::span(kMarketDisplayGenerated));
untrusted_source->AddResourcePaths(
base::make_span(kMarketDisplayGenerated, kMarketDisplayGeneratedSize));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
webui::SetupWebUIDataSource(untrusted_source,
base::span(kMarketDisplayGenerated),
IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML);
webui::SetupWebUIDataSource(
untrusted_source,
base::make_span(kMarketDisplayGenerated, kMarketDisplayGeneratedSize),
IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML);

untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ScriptSrc,
Expand Down
16 changes: 12 additions & 4 deletions browser/ui/webui/brave_wallet/nft/nft_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifdef UNSAFE_BUFFERS_BUILD
// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and
// convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif

#include "brave/browser/ui/webui/brave_wallet/nft/nft_ui.h"

#include <string>
Expand Down Expand Up @@ -33,12 +39,14 @@ UntrustedNftUI::UntrustedNftUI(content::WebUI* web_ui)
}

untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_NFT_DISPLAY_HTML);
untrusted_source->AddResourcePaths(base::span(kNftDisplayGenerated));
untrusted_source->AddResourcePaths(
base::make_span(kNftDisplayGenerated, kNftDisplayGeneratedSize));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
webui::SetupWebUIDataSource(untrusted_source,
base::span(kNftDisplayGenerated),
IDR_BRAVE_WALLET_NFT_DISPLAY_HTML);
webui::SetupWebUIDataSource(
untrusted_source,
base::make_span(kNftDisplayGenerated, kNftDisplayGeneratedSize),
IDR_BRAVE_WALLET_NFT_DISPLAY_HTML);
untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ScriptSrc,
std::string("script-src 'self' chrome-untrusted://resources;"));
Expand Down
3 changes: 2 additions & 1 deletion browser/ui/webui/brave_wallet/trezor/trezor_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ UntrustedTrezorUI::UntrustedTrezorUI(content::WebUI* web_ui)
auto* untrusted_source = content::WebUIDataSource::CreateAndAdd(
web_ui->GetWebContents()->GetBrowserContext(), kUntrustedTrezorURL);
untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_TREZOR_BRIDGE_HTML);
untrusted_source->AddResourcePaths(base::span(kTrezorBridgeGenerated));
untrusted_source->AddResourcePaths(UNSAFE_TODO(
base::make_span(kTrezorBridgeGenerated, kTrezorBridgeGeneratedSize)));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
untrusted_source->OverrideContentSecurityPolicy(
Expand Down
7 changes: 5 additions & 2 deletions browser/ui/webui/brave_wallet/wallet_page_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ WalletPageUI::WalletPageUI(content::WebUI* web_ui)
"braveWalletPendingTransactions", IDS_BRAVE_WALLET_PENDING_TRANSACTIONS);
web_ui->AddMessageHandler(std::move(plural_string_handler));
NavigationBarDataProvider::Initialize(source, profile);
webui::SetupWebUIDataSource(source, base::span(kBraveWalletPageGenerated),
IDR_WALLET_PAGE_HTML);
webui::SetupWebUIDataSource(
source,
UNSAFE_TODO(base::make_span(kBraveWalletPageGenerated,
kBraveWalletPageGeneratedSize)),
IDR_WALLET_PAGE_HTML);
source->AddString("braveWalletLedgerBridgeUrl", kUntrustedLedgerURL);
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ImgSrc,
Expand Down
7 changes: 5 additions & 2 deletions browser/ui/webui/brave_wallet/wallet_panel_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ WalletPanelUI::WalletPanelUI(content::WebUI* web_ui)
plural_string_handler->AddLocalizedString(
"braveWalletPendingTransactions", IDS_BRAVE_WALLET_PENDING_TRANSACTIONS);
web_ui->AddMessageHandler(std::move(plural_string_handler));
webui::SetupWebUIDataSource(source, base::span(kBraveWalletPanelGenerated),
IDR_WALLET_PANEL_HTML);
webui::SetupWebUIDataSource(
source,
UNSAFE_TODO(base::make_span(kBraveWalletPanelGenerated,
kBraveWalletPanelGeneratedSize)),
IDR_WALLET_PANEL_HTML);
source->AddString("braveWalletLedgerBridgeUrl", kUntrustedLedgerURL);
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::FrameSrc,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifdef UNSAFE_BUFFERS_BUILD
// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and
// convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif

#include "brave/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h"

#include <stdint.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BITCOIN_BITCOIN_DISCOVER_ACCOUNT_TASK_H_
#define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BITCOIN_BITCOIN_DISCOVER_ACCOUNT_TASK_H_

#include <memory>
#include <string>
#include <utility>

#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
Expand Down Expand Up @@ -76,7 +74,7 @@ class DiscoverAccountTaskBase {
uint32_t active_requests_ = 0;
// Indexed by 0 and 1 for receive and change addresses discovery states
// respectively.
std::array<State, 2> states_;
State states_[2];
bool account_is_used_ = false;
mojom::BitcoinBalancePtr balance_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <utility>

#include "base/containers/span.h"
#include "base/rand_util.h"
#include "base/strings/string_number_conversions.h"
#include "brave/components/brave_wallet/browser/bitcoin/bitcoin_hd_keyring.h"
Expand Down Expand Up @@ -75,7 +74,7 @@ class BitcoinKnapsackSolverUnitTest : public testing::Test {
tx_input.utxo_address = address;
std::string txid_fake = address + base::NumberToString(amount);
tx_input.utxo_outpoint.txid =
crypto::SHA256Hash(base::as_byte_span(txid_fake));
crypto::SHA256Hash(base::as_bytes(base::make_span(txid_fake)));
tx_input.utxo_outpoint.index = tx_input.utxo_outpoint.txid.back();
tx_input.utxo_value = amount;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class BitcoinMaxSendSolverUnitTest : public testing::Test {
tx_input.utxo_address = address;
std::string txid_fake = address + base::NumberToString(amount);
tx_input.utxo_outpoint.txid =
crypto::SHA256Hash(base::as_byte_span(txid_fake));
crypto::SHA256Hash(base::as_bytes(base::make_span(txid_fake)));
tx_input.utxo_outpoint.index = tx_input.utxo_outpoint.txid.back();
tx_input.utxo_value = amount;

Expand Down
19 changes: 11 additions & 8 deletions components/brave_wallet/browser/eip1559_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <optional>
#include <utility>

#include "base/containers/extend.h"
#include "base/values.h"
#include "brave/components/brave_wallet/browser/rlp_encode.h"
#include "brave/components/brave_wallet/common/hash_utils.h"
Expand Down Expand Up @@ -271,9 +270,11 @@ std::optional<Eip1559Transaction> Eip1559Transaction::FromValue(
return tx;
}

std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(
uint256_t chain_id) const {
std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(uint256_t chain_id,
bool hash) const {
DCHECK(nonce_);
std::vector<uint8_t> result;
result.push_back(type_);

base::Value::List list;
list.Append(RLPUint256ToBlob(chain_id_));
Expand All @@ -286,10 +287,9 @@ std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(
list.Append(base::Value(data_));
list.Append(base::Value(AccessListToValue(access_list_)));

std::vector<uint8_t> result;
result.push_back(type_);
base::Extend(result, RLPEncode(list));
return result;
const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
return hash ? KeccakHash(result) : result;
}

std::string Eip1559Transaction::GetSignedTransaction() const {
Expand Down Expand Up @@ -361,7 +361,10 @@ std::vector<uint8_t> Eip1559Transaction::Serialize() const {

std::vector<uint8_t> result;
result.push_back(type_);
base::Extend(result, RLPEncode(list));

const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());

return result;
}

Expand Down
7 changes: 4 additions & 3 deletions components/brave_wallet/browser/eip1559_transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ class Eip1559Transaction : public Eip2930Transaction {
gas_estimation_ = estimation;
}

// 0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas,
// gasLimit, destination, value, data, access_list])
std::vector<uint8_t> GetMessageToSign(uint256_t chain_id) const override;
// keccak256(0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas,
// gasLimit, destination, value, data, access_list]))
std::vector<uint8_t> GetMessageToSign(uint256_t chain_id = 0,
bool hash = true) const override;

// 0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit,
// destination, value, data, accessList, signatureYParity, signatureR,
Expand Down
15 changes: 7 additions & 8 deletions components/brave_wallet/browser/eip1559_transaction_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <optional>
#include <utility>

#include "base/containers/span.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/values.h"
Expand Down Expand Up @@ -50,7 +49,7 @@ TEST(Eip1559TransactionUnitTest, GetMessageToSign) {

access_list->push_back(item);

EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetHashedMessageToSign(0))),
EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetMessageToSign())),
"fa81814f7dd57bad435657a05eabdba2815f41e3f15ddd6139027e7db56b0dea");
}

Expand Down Expand Up @@ -126,10 +125,10 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) {
"0x863c02549182b91f1764714b93d7e882f010539c0907adaf4de761f7b06a713c"}};
for (const auto& entry : cases) {
SCOPED_TRACE(entry.signed_tx);
std::array<uint8_t, 32> private_key;
EXPECT_TRUE(base::HexStringToSpan(
std::vector<uint8_t> private_key;
EXPECT_TRUE(base::HexStringToBytes(
"8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63",
private_key));
&private_key));

HDKey key;
key.SetPrivateKey(private_key);
Expand All @@ -143,9 +142,9 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) {
nullptr));

int recid;
auto signature = key.SignCompact(tx.GetHashedMessageToSign(0), &recid);
ASSERT_TRUE(signature);
tx.ProcessSignature(*signature, recid, 0);
const std::vector<uint8_t> signature =
key.SignCompact(tx.GetMessageToSign(), &recid);
tx.ProcessSignature(signature, recid);
EXPECT_EQ(tx.GetSignedTransaction(), entry.signed_tx);
EXPECT_EQ(tx.GetTransactionHash(), entry.hash);
}
Expand Down
21 changes: 12 additions & 9 deletions components/brave_wallet/browser/eip2930_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <optional>
#include <utility>

#include "base/containers/extend.h"
#include "base/values.h"
#include "brave/components/brave_wallet/browser/rlp_encode.h"
#include "brave/components/brave_wallet/common/eth_address.h"
Expand Down Expand Up @@ -162,9 +161,11 @@ Eip2930Transaction::ValueToAccessList(const base::Value::List& value) {
return access_list;
}

std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(
uint256_t chain_id) const {
std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(uint256_t chain_id,
bool hash) const {
DCHECK(nonce_);
std::vector<uint8_t> result;
result.push_back(type_);

base::Value::List list;
list.Append(RLPUint256ToBlob(chain_id_));
Expand All @@ -176,10 +177,9 @@ std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(
list.Append(base::Value(data_));
list.Append(base::Value(AccessListToValue(access_list_)));

std::vector<uint8_t> result;
result.push_back(type_);
base::Extend(result, RLPEncode(list));
return result;
const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
return hash ? KeccakHash(result) : result;
}

std::string Eip2930Transaction::GetSignedTransaction() const {
Expand All @@ -196,7 +196,7 @@ std::string Eip2930Transaction::GetTransactionHash() const {
return ToHex(KeccakHash(Serialize()));
}

void Eip2930Transaction::ProcessSignature(base::span<const uint8_t> signature,
void Eip2930Transaction::ProcessSignature(const std::vector<uint8_t> signature,
int recid,
uint256_t chain_id) {
EthTransaction::ProcessSignature(signature, recid, chain_id_);
Expand Down Expand Up @@ -241,7 +241,10 @@ std::vector<uint8_t> Eip2930Transaction::Serialize() const {

std::vector<uint8_t> result;
result.push_back(type_);
base::Extend(result, RLPEncode(list));

const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());

return result;
}

Expand Down
Loading

0 comments on commit c728965

Please sign in to comment.