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

Merge dependabot security PRs #4847

Open
mikesposito opened this issue Oct 24, 2024 · 0 comments
Open

Merge dependabot security PRs #4847

mikesposito opened this issue Oct 24, 2024 · 0 comments
Assignees
@mcmire @mikesposito
Labels
team-wallet-framework

Comments

@mikesposito
Copy link
Member

mikesposito commented Oct 24, 2024 •
edited
Loading

There are several PR related to security vulnerabilities from dependabot in repos owned by Wallet Framework (for full list, see https://github.com/MetaMask/MetaMask-planning/issues/3540).

In some cases, we should also prioritize release and update of affected packages in their consumers in order to mitigate the security issues, based on their EPSS value.

When to release the package

  • if EPSS is >= 1% then release the package and deliver to clients
  • if EPSS is < 1%
    • if the package is released (and delivered to clients) frequently then just merge the dependabot PR
    • if the package is rarely updated, release and deliver to (at least) other packages that are released more frequently, or to clients if it makes sense

To get the EPSS value

For packages we are in process of archiving
For affected packages that are in the process of being abandoned/archived, we should make sure that the related vulnerabilities have been mitigated on clients before ignoring the dependabot PR.

PR List
@mcmire mcmire self-assigned this Nov 26, 2024
@cryptodev-2s cryptodev-2s removed their assignment Dec 4, 2024
@desi desi assigned mikesposito and unassigned MajorLift and mikesposito Dec 5, 2024
@mikesposito mikesposito mentioned this issue Dec 6, 2024
Merged
4 tasks
This was referenced Jan 14, 2025
Merged
Merged
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this issue Feb 6, 2025
@mikesposito @metamaskbot
chore(deps): update express (#29708)
ffdbd3b 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This is a lock-only change to update `express` to a version higher than
`4.19.2`, according to this security advisory:
https://github.com/MetaMask/metamask-extension/security/dependabot/157

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29708?quickstart=1)

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this issue Feb 11, 2025
@mikesposito @Gudahtt
chore(deps): bump elliptic to ^6.6.0 (#12979)
fd9da65 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This PR bumps `elliptic` in the dependency tree to mitigate the
following security advisories:
- https://github.com/MetaMask/metamask-mobile/security/dependabot/154
- https://github.com/MetaMask/metamask-mobile/security/dependabot/146
- https://github.com/MetaMask/metamask-mobile/security/dependabot/129
- https://github.com/MetaMask/metamask-mobile/security/dependabot/128
- https://github.com/MetaMask/metamask-mobile/security/dependabot/127
    
The closer version that mitigates all the above advisories is `6.6.0`. 

Currently on `main`, these are the versions we have for `elliptic`:
```bash
> yarn why elliptic
=> Found "[email protected]"
info Has been hoisted to "elliptic"
info Reasons this module exists
   - Hoisted from "@WalletConnect#utils#elliptic"
   - Hoisted from "@MetaMask#ppom-validator#elliptic"
   - Hoisted from "secp256k1#elliptic"
   - Hoisted from "ethereumjs-abi#ethereumjs-util#elliptic"
   - Hoisted from "react-native-crypto#create-ecdh#elliptic"
   - Hoisted from "react-native-crypto#browserify-sign#elliptic"
   - Hoisted from "ethereumjs-util#secp256k1#elliptic"
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@ethersproject/signing-key#[email protected]"
info This module exists because "ethers#@ethersproject#signing-key" depends on it.
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@reown/walletkit#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
=> Found "@walletconnect/sign-client#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
```

The added resolution forces the package on `^6.6.0` which currently
resolves to `6.6.1`

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: Mark Stacey <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this issue Feb 11, 2025
@mikesposito @Gudahtt
chore(deps): bump elliptic to ^6.6.0 (#12979)
d17ffb4 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This PR bumps `elliptic` in the dependency tree to mitigate the
following security advisories:
- https://github.com/MetaMask/metamask-mobile/security/dependabot/154
- https://github.com/MetaMask/metamask-mobile/security/dependabot/146
- https://github.com/MetaMask/metamask-mobile/security/dependabot/129
- https://github.com/MetaMask/metamask-mobile/security/dependabot/128
- https://github.com/MetaMask/metamask-mobile/security/dependabot/127
    
The closer version that mitigates all the above advisories is `6.6.0`. 

Currently on `main`, these are the versions we have for `elliptic`:
```bash
> yarn why elliptic
=> Found "[email protected]"
info Has been hoisted to "elliptic"
info Reasons this module exists
   - Hoisted from "@WalletConnect#utils#elliptic"
   - Hoisted from "@MetaMask#ppom-validator#elliptic"
   - Hoisted from "secp256k1#elliptic"
   - Hoisted from "ethereumjs-abi#ethereumjs-util#elliptic"
   - Hoisted from "react-native-crypto#create-ecdh#elliptic"
   - Hoisted from "react-native-crypto#browserify-sign#elliptic"
   - Hoisted from "ethereumjs-util#secp256k1#elliptic"
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@ethersproject/signing-key#[email protected]"
info This module exists because "ethers#@ethersproject#signing-key" depends on it.
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@reown/walletkit#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
=> Found "@walletconnect/sign-client#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
```

The added resolution forces the package on `^6.6.0` which currently
resolves to `6.6.1`

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: Mark Stacey <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this issue Feb 11, 2025
@mikesposito @Gudahtt
chore(deps): bump elliptic to ^6.6.0 (#12979)
e00b97f 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This PR bumps `elliptic` in the dependency tree to mitigate the
following security advisories:
- https://github.com/MetaMask/metamask-mobile/security/dependabot/154
- https://github.com/MetaMask/metamask-mobile/security/dependabot/146
- https://github.com/MetaMask/metamask-mobile/security/dependabot/129
- https://github.com/MetaMask/metamask-mobile/security/dependabot/128
- https://github.com/MetaMask/metamask-mobile/security/dependabot/127
    
The closer version that mitigates all the above advisories is `6.6.0`. 

Currently on `main`, these are the versions we have for `elliptic`:
```bash
> yarn why elliptic
=> Found "[email protected]"
info Has been hoisted to "elliptic"
info Reasons this module exists
   - Hoisted from "@WalletConnect#utils#elliptic"
   - Hoisted from "@MetaMask#ppom-validator#elliptic"
   - Hoisted from "secp256k1#elliptic"
   - Hoisted from "ethereumjs-abi#ethereumjs-util#elliptic"
   - Hoisted from "react-native-crypto#create-ecdh#elliptic"
   - Hoisted from "react-native-crypto#browserify-sign#elliptic"
   - Hoisted from "ethereumjs-util#secp256k1#elliptic"
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@ethersproject/signing-key#[email protected]"
info This module exists because "ethers#@ethersproject#signing-key" depends on it.
info Disk size without dependencies: "288KB"
info Disk size with unique dependencies: "656KB"
info Disk size with transitive dependencies: "656KB"
info Number of shared dependencies: 7
=> Found "@reown/walletkit#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
=> Found "@walletconnect/sign-client#[email protected]"
info Reasons this module exists
   - "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils" depends on it
   - Hoisted from "@WalletConnect#se-sdk#@Reown#walletkit#@WalletConnect#sign-client#@WalletConnect#utils#elliptic"
info Disk size without dependencies: "172KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 7
```

The added resolution forces the package on `^6.6.0` which currently
resolves to `6.6.1`

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: Mark Stacey <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this issue Feb 14, 2025
@mikesposito @metamaskbot @mcmire
chore(deps): update express (#29708)
e7bbf08 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This is a lock-only change to update `express` to a version higher than
`4.19.2`, according to this security advisory:
https://github.com/MetaMask/metamask-extension/security/dependabot/157

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29708?quickstart=1)

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
PatrykLucka pushed a commit to MetaMask/metamask-extension that referenced this issue Feb 19, 2025
@mikesposito @metamaskbot
@mcmire @PatrykLucka
chore(deps): update express (#29708)
8d47e8e 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This is a lock-only change to update `express` to a version higher than
`4.19.2`, according to this security advisory:
https://github.com/MetaMask/metamask-extension/security/dependabot/157

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29708?quickstart=1)

## **Related issues**

Related: MetaMask/core#4847

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mcmire mcmire

@mikesposito mikesposito

Labels
team-wallet-framework
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mcmire @MajorLift @mikesposito @kanthesha @cryptodev-2s