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.

Sign up for GitHub

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

Jump to bottom

feat: Ledger team request us to upgrade the @ledgerhq/hw-app-eth to 6.42.0 to fix ledger bug for EIP-712 content #29820

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

feat: Ledger team request us to upgrade the @ledgerhq/hw-app-eth to 6.42.0 to fix ledger bug for EIP-712 content #29820

wants to merge 7 commits into from
+109 −212

Conversation

dawnseeker8
Copy link
Contributor

@dawnseeker8 dawnseeker8 commented Jan 21, 2025 •
edited
Loading

Description

Ledger team request us to upgrade the @ledgerhq/hw-app-eth to 6.42.0 to fix ledger bug for EIP-712 content
Here is some comment from Kevin LAMBERT from ledger team:

and this is original thread https://consensys.slack.com/archives/C02CYKAA8G1/p1737132760664329?thread_ts=1737106010.543919&cid=C02CYKAA8G1

Open in GitHub Codespaces

Related issues

Fixes: #29813

Manual testing steps

Will require a full regression test for ledger feature.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions GitHub Actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@socket-security Socket Security
Copy link

socket-security bot commented Jan 21, 2025 •
edited
Loading

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
AI-detected potential security risk npm/@ledgerhq/[email protected]
  • Notes: The code's purpose and behavior cannot be conclusively determined without decoding. However, the use of Base64 encoding raises concerns about the potential for malicious intent. Proceed with caution if the string is executed or used in any application.
  • Confidence: 0.90
  • Severity: 0.65
 🚫
AI-detected potential security risk npm/@ledgerhq/[email protected]
  • Notes: The source code is base64 encoded, suggesting potential obfuscation. The reports are incomplete, providing no actionable insights. Decoding is necessary for a thorough security assessment. Estimated scores reflect the uncertainty and potential risk due to the encoded nature.
  • Confidence: 0.80
  • Severity: 0.60
 🚫

Ignoring: npm/[email protected]

View full report↗︎

Next steps

What are AI-detected potential security risks?

AI has determined that this package may contain potential security issues or vulnerabilities.

An AI system identified potential security problems in this package. It is advised to review the package thoroughly and assess the potential risks before installation. You may also consider reporting the issue to the package maintainer or seeking alternative solutions with a stronger security posture.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@dawnseeker8
Copy link
Contributor Author

@SocketSecurity ignore npm/[email protected]

@dawnseeker8
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot
Copy link
Collaborator

Builds ready [745a410]
Page Load Metrics (1558 ± 88 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint13602001155718086
domContentLoaded13511987154217685
load13622000155818488
domInteractive237034146
backgroundConnect64813126
firstReactRender14103282411
getState447894
initialActions00000
loadScripts9361491110614570
setupStore55815168
uiStartup153726901790265127
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -12.21 KiB (-0.21%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [53d360c]
Page Load Metrics (1486 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1395164714968038
domContentLoaded1357163514677335
load1395165914868239
domInteractive225632115
backgroundConnect65119157
firstReactRender1593393014
getState47013188
initialActions01000
loadScripts999123210816029
setupStore585232713
uiStartup15302187179820799
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -14.06 KiB (-0.24%)
  • ui: -105 Bytes (-0.00%)
  • common: 73.09 KiB (0.84%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
team-hardware-wallets
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Upgrade hw-app-eth library to v6.42.0 for Ledger Clear Signing
2 participants
@dawnseeker8 @metamaskbot