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

[New Snap] Massa #990

Open
7 tasks
khanti42 opened this issue Jan 6, 2025 · 0 comments · May be fixed by #987
Open
7 tasks

[New Snap] Massa #990

khanti42 opened this issue Jan 6, 2025 · 0 comments · May be fixed by #987

Comments

@khanti42
Copy link
Collaborator

khanti42 commented Jan 6, 2025

Checklist

All items in the list below needs to be satisfied.

  • Is the Snap repository publicly accessible and linked in this ticket: https://github.com/massalabs/metamask-massa
  • Is the Snap distributed on npm and linked in this ticket: https://www.npmjs.com/package/@massalabs/metamask-snap
  • Has an audit been performed and the audit report attached or linked in this issue:
  • Is a complete list of discovered vulnerabilities from the audit documented in this issue?
  • For vulnerabilities that have been deemed necessary to be addressed, are the links to the fixes attached to this issue?
  • For vulnerabilities that have been deemed not necessary to be addressed, is a reason for each of them documented in this issue?
  • The corresponding pull request in this repo has been merged.

Changes since audit

  • 67c88e0 - use m/44/632 key deriver path
  • 54b4c8b - add Divider in homepage component
  • ffcbd08 - add logo in npm files
  • 97d5299 - add Mas send in snap ui

Audit details

Report: https://sayfer.io/audits/metamask-snap-audit-report-for-massa/
Audit commit: 07G95fc8fed4f219b810702804ba74051079577d
Fixes commit: 92096a47e47ef9fbe9c45b9623464fa7482bf8ab

1. Custom Networks Won’t Function Correctly

  • ID: SAY-01
  • Risk: Medium
  • Status: Fixed
  • Description:
    The snap does not support adding custom networks properly. Functions like getClient(bigint) and getNodeUrls() only support the default networks (MAINNET and BUILDNET). Custom networks result in fallback to BUILDNET.
  • Mitigation:
    Implement a mechanism to allow users to set a custom provider for networks other than MAINNET and BUILDNET.

2. Incorrect Return Value

  • ID: SAY-02
  • Risk: Medium
  • Status: Fixed
  • Description:
    The getBalance(GetBalanceParams) function assigns the same value to both finalBalance and candidateBalance, leading to inaccurate balances displayed to users.
  • Mitigation:
    Ensure that finalBalance and candidateBalance return distinct and correct values.

3. Duplicate Operations Are Not Saved

  • ID: SAY-03
  • Risk: Low
  • Status: Fixed
  • Description:
    The code prevents duplicate operations from being saved, causing users to miss seeing repeated actions in their operation history.
  • Mitigation:
    Remove the check that skips duplicate operations, allowing them to be saved in memory.

4. Roll Operations Are Not Saved to Storage

  • ID: SAY-04
  • Risk: Low
  • Status: Fixed
  • Description:
    Operations related to roll transactions (e.g., buy and sell rolls) are not saved to storage, making them invisible in operation history.
  • Mitigation:
    Add logic to save roll operations to memory using addAccountOperation.

5. Imprecise Method Names

  • ID: SAY-05
  • Risk: Informational
  • Status: Fixed
  • Description:
    Methods like listAccounts() and getNodeUrls() return single objects instead of multiple ones, contrary to their names.
  • Mitigation:
    Adjust function names to better reflect their behavior.

6. No Parameter Prefix Validation

  • ID: SAY-06
  • Risk: Informational
  • Status: Fixed
  • Description:
    The codebase comments specify that prefixes (e.g., AU for addresses, OP for operation IDs) should be validated, but no actual checks are implemented.
  • Mitigation:
    Add validation checks for prefixes if intended, or clarify the comments.

7. Function May Fail If the Number of Operations Is Too Large

  • ID: SAY-07
  • Risk: Informational
  • Status: Fixed
  • Description:
    The getOperations() function might fail if the number of stored operations grows excessively, potentially overloading memory.
  • Mitigation:
    Introduce a limit to the number of stored or returned operations and consider removing older entries once the limit is exceeded.

Summary

  • Total Vulnerabilities Identified: 7
    • Medium Risk: 2
    • Low Risk: 2
    • Informational: 3
  • Status: All issues have been fixed by the Massa team, ensuring a competent security posture for their MetaMask Snap.
@khanti42 khanti42 linked a pull request Jan 6, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant