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] Move Wallet #1096

Open
7 tasks
khanti42 opened this issue Feb 26, 2025 · 0 comments · May be fixed by #1097
Open
7 tasks

[New Snap] Move Wallet #1096

khanti42 opened this issue Feb 26, 2025 · 0 comments · May be fixed by #1097

Comments

@khanti42
Copy link
Collaborator

khanti42 commented Feb 26, 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/nightly-labs/move-snap
  • Is the Snap distributed on npm and linked in this ticket: https://www.npmjs.com/package/@nightlylabs/metamask-move-snap
  • Has an audit been performed and the audit report attached or linked in this issue: yes
  • 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.

Audit details

Audit Conducted by: OtterSec
Audit Report: link
Audit Date: January 16-17, 2025
Repository: aptos-snap
Commit Audited: 0b381a1
Total Findings: 3 (2 vulnerabilities, 1 general finding)

The repo linked in this issue is now https://github.com/nightly-labs/move-snap but it's a copy of the aptos-snap. And its the new name moving forward but the audit was conducted when the repo was named aptos-snap.


Findings

1️⃣ Lack of Transaction Transparency (OS-NLB-ADV-00)

  • Severity: ⚠️ Medium
  • Description: Transaction parameters were not explicitly displayed to users before signing, increasing the risk of fraudulent transactions.
  • Resolution: ✅ Fixed in commit de5a944.
  • Remediation: Implemented transaction parameter display and committed to full transaction simulation.

2️⃣ Improper Input Sanitization (OS-NLB-ADV-01)

  • Severity: ⚠️ Low
  • Description: The Snap module did not properly handle newline characters (\n or \r) in dialog messages, allowing potential phishing or spoofing attacks.
  • Resolution: ✅ Fixed in commit 7911177.
  • Remediation: Implemented input sanitization to strip newline characters.

3️⃣ Validation of Network URL Format (OS-NLB-SUG-00)

  • Severity: ℹ️ Informational
  • Description: Malformed URLs for network configurations could bypass verification, posing security risks.
  • Recommendation: Implement strict validation using new URL() to ensure valid and structured network URLs.

✅ Resolution Status

  • 🟢 All vulnerabilities have been resolved.
  • 🟡 General recommendation for URL validation is suggested but not an immediate security concern.
@khanti42 khanti42 linked a pull request Feb 26, 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