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

Add Safe Transaction Details Parser Feature #33

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shayzluf
Copy link

@shayzluf shayzluf commented Mar 14, 2025

Summary

This PR introduces a new feature that allows users to paste transaction details copied directly from the Safe UI and automatically populate the transaction form fields. This significantly streamlines the process of recreating or analyzing Safe transactions.

Problem Solved

Previously, users needed to manually input transaction details field by field, which was time-consuming and error-prone. This feature simplifies the workflow by:

  1. Allowing direct copy-paste from Safe UI transaction details
  2. Automatically parsing and extracting relevant transaction information
  3. Populating form fields with the extracted data
  4. Detecting special cases like truncated content and missing fields

The tool acknowledges that user validation of transaction details remains essential regardless of input method - whether manual entry or copy-paste - and encourages users to verify critical transaction information before proceeding.

Implementation

Key Components:

  1. PasteTransactionDetails Component:

    • Provides a dedicated interface for pasting transaction text
    • Handles parsing and form population
    • Provides visual feedback on success, warnings, and errors
  2. Transaction Text Parser:

    • Extracts transaction fields using regex patterns
    • Detects field formats from various versions of Safe UI
    • Handles special cases like shorthand addresses and EIP-3770 chain shortnames
  3. Smart Detection Features:

    • Network detection from chain shortnames (e.g., "eth:", "gnosis:")
    • Truncated field detection with user guidance
    • Missing fields identification to guide manual completion
  4. User Experience Enhancements:

    • Visual indicators for successful parsing, warnings, and errors
    • Clipboard paste button for convenience
    • Clear guidance when manual intervention is needed

How to Test

Basic Functionality:

  1. Open a Safe transaction in Safe UI
  2. Select and copy the transaction details
Screenshot 2025-03-14 at 9 51 33
  1. Paste the copied text into our tool's paste area
  2. Click "Parse and Fill Form"
Screenshot 2025-03-14 at 9 49 10
  1. Verify that the form fields are correctly populated with transaction details

Network Detection:

  1. Copy a transaction that includes a chain shortname (e.g., "eth:" or "gnosis:")
  2. Paste and parse
  3. Verify that the correct network is automatically selected

Handling Edge Cases:

  • Test with truncated data fields (tool should warn appropriately)
  • Test with incomplete information (tool should identify missing fields)
  • Test with different Safe UI versions (tool should handle parsing variations)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

github-actions bot commented Mar 14, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@shayzluf
Copy link
Author

shayzluf commented Mar 14, 2025

I have read the CLA Document and I hereby sign the CLA

@shayzluf
Copy link
Author

recheck

@josepchetrit12
Copy link
Collaborator

josepchetrit12 commented Mar 14, 2025

@shayzluf thanks for your contribution.

I like the idea of trying to minimize the data that you need to input, but this can be dangerous for the user if the UI is compromised again.

We also discussed with Safe team to support url params, so you don't need to input anything. But we are not sure about that, we really want the user to input the params so we make sure they understand what they are copy/paste.

Imagine if the safe ui is compromised again, when you're doing this copy paste you're understanding more the Tx at least that's my opinion.

If you want to enter less data you always have the safe api method.

What do you think about that?

@shayzluf
Copy link
Author

@shayzluf thanks for your contribution.

I like the idea of trying to minimize the data that you need to input, but this can be dangerous for the user if the UI is compromised again.

We also discussed with Safe team to support url params, so you don't need to input anything. But we are not sure about that, we really want the user to input the params so we make sure they understand what they are copy/paste.

Imagine if the safe ui is compromised again, when you're doing this copy paste you're understanding more the Tx at least that's my opinion.

If you want to enter less data you always have the safe api method.

What do you think about that?

Thank you for your thoughtful feedback. I understand your security concerns regarding copy-paste functionality, but I'd like to offer some context about how this feature might actually help in certain security scenarios.

Regarding the specific Safe UI compromise you mentioned: In that attack, the UI showed users the correct transaction details (which they verified visually) but sent different parameters to be signed behind the scenes. In such a scenario, this feature would actually provide an additional security layer since it works by parsing the visible text content that users see on screen rather than relying on hidden parameters.

My initial concept was even more security-focused: I explored implementing OCR on an air-gapped mobile device that would scan the desktop screen and populate the fields using the decoded text. Unfortunately, technical limitations with OCR accuracy (approximately one character error per address) made this approach impractical.

This current implementation is primarily a UX improvement that:

  1. Doesn't introduce vulnerabilities beyond what already exists when users copy-paste individual fields
  2. Encourages users to validate all fields with clear visual indicators for missing or truncated data
  3. Actually improves security in scenarios where what's displayed differs from what's submitted

You make an excellent point that user understanding is critical. This tool strikes a balance by improving usability while still requiring user validation. Users still need to review and verify the populated fields before signing - the same verification step needed with manual entry - but with significantly reduced tedium.

Regarding the Safe API method, while it's a good alternative, many users prefer the transparency of directly seeing and managing transaction parameters, which this feature facilitate

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

Successfully merging this pull request may close these issues.

None yet

2 participants