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

[Bug]: vm2 is deprecated for critical security issue #10547

Closed
matthew-gerstman opened this issue Jul 12, 2023 · 8 comments · Fixed by #10548
Closed

[Bug]: vm2 is deprecated for critical security issue #10547

matthew-gerstman opened this issue Jul 12, 2023 · 8 comments · Fixed by #10548

Comments

@matthew-gerstman
Copy link

Minimal, reproducible example

npm install -g puppeteer   
npm WARN deprecated vm2@3.9.19: The library contains critical security issues and should not be used for production! The maintenance of the project has been discontinued. Consider migrating your code to isolated-vm.

It looks like a downstream dependency has a critical security issue and needs to be updated. Is it a big lift to update to isolated-vm?



### Error string

The library contains critical security issues and should not be used for production!

### Bug behavior

- [ ] Flaky
- [ ] PDF

### Background

_No response_

### Expectation

Not have a critical security issue

### Reality

It has a critical security issue

### Puppeteer configuration

_No response_

### Puppeteer version

20.8.1

### Node version

16.18.0

### Package manager

npm

### Package manager version

8.19.2

### Operating system

macOS
@github-actions
Copy link

This issue was not reproducible. Please check that your example runs locally and the following:

  • Ensure the script does not rely on dependencies outside of puppeteer and puppeteer-core.
  • Ensure the error string is just the error message.
    • Bad:

      Error: something went wrong
        at Object.<anonymous> (/Users/username/repository/script.js:2:1)
        at Module._compile (node:internal/modules/cjs/loader:1159:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
        at Module.load (node:internal/modules/cjs/loader:1037:32)
        at Module._load (node:internal/modules/cjs/loader:878:12)
        at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
        at node:internal/main/run_main_module:23:47
    • Good: Error: something went wrong.

  • Ensure your configuration file (if applicable) is valid.
  • If the issue is flaky (does not reproduce all the time), make sure 'Flaky' is checked.
  • If the issue is not expected to error, make sure to write 'no error'.

Once the above checks are satisfied, please edit your issue with the changes and we will
try to reproduce the bug again.


Analyzer run

@jcmaunsell
Copy link

See patriksimek/vm2#533:

Do to security issues that cannot be properly addressed I (XmiliaH) will stop maintaining this library.
For a replacement look into isolated-vm.

The problems that piled up are caused by node intercepting calls from the sandbox and therefore arguments not being wrapped in a Proxy. As isolated-vm does create the sandbox on their own and does not rely on the vm module there shouldn't be hooks added by node that causes such issues.

@OrKoN
Copy link
Collaborator

OrKoN commented Jul 12, 2023

we do not directly depend on this but via the proxy-agents package that probably needs it to support PAC file proxies that can be for downloading of the browser binaries.

@OrKoN
Copy link
Collaborator

OrKoN commented Jul 12, 2023

See TooTallNate/proxy-agents#218

@jcmaunsell
Copy link

Thanks - but could this transitive dependency still pose a security issue for Puppeteer?

@OrKoN
Copy link
Collaborator

OrKoN commented Jul 12, 2023

@jcmaunsell unlikely given that it is only used to process proxy settings (that are set by you as a user or your system administrator) and it is only used during installation to download browser binaries.

@OrKoN
Copy link
Collaborator

OrKoN commented Jul 12, 2023

anyway we will likely stop supporting pac file proxies as there seem to be no good solution for this.

@matthew-gerstman
Copy link
Author

Thanks so much for the quick turnaround here!

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.

3 participants