Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UNSAFE_TODO for wallet [part 1 of N] #26431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

supermassive
Copy link
Collaborator

Resolves

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Copy link
Contributor

github-actions bot commented Nov 7, 2024

[puLL-Merge] - brave/brave-core@26431

Description

This PR primarily addresses the removal of unsafe buffer usage and replaces it with safer constructs across various components of the Brave Wallet. The changes mainly involve updating function calls to use base::span instead of base::make_span, removing UNSAFE_TODO macros, and refactoring some buffer handling code.

Changes

Changes

  1. browser/ui/webui/brave_wallet/

    • Updated multiple files to use base::span instead of base::make_span
    • Removed UNSAFE_TODO macros and #pragma allow_unsafe_buffers directives
    • Simplified resource loading code in various UI components
  2. components/brave_wallet/browser/

    • Updated bitcoin_discover_account_task.h to use std::array instead of C-style array
    • Modified solana_instruction_builder.cc to use base::span and refactored UintToLEBytes function
    • Updated solana_message.cc and solana_message_address_table_lookup.cc to use base::span
    • Refactored wallet_data_files_installer.cc to use safer array assignment
  3. components/brave_wallet/browser/bitcoin/

    • Updated bitcoin_knapsack_solver_unittest.cc and bitcoin_max_send_solver_unittest.cc to use base::as_byte_span
  4. components/brave_wallet/browser/permission_utils_unittest.cc

    • Refactored test cases to use range-based for loops and remove C-style array access
  5. components/brave_wallet/browser/sns_resolver_task.cc

    • Updated to use base::span and base::as_byte_span

Possible Issues

  • The changes in buffer handling might introduce subtle differences in behavior, especially in edge cases. Thorough testing is recommended to ensure no regressions occur.

Security Hotspots

No significant security hotspots were identified in this change. The modifications generally improve safety by replacing unsafe buffer usage with safer alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant