protobufjs / protobuf.js

Protocol Buffers for JavaScript & TypeScript.
Other
9.77k stars 1.4k forks source link

Use of eval is strongly discouraged #1754

Open imoye opened 2 years ago

imoye commented 2 years ago

protobuf.js version: 6.10.2

https://rollupjs.org/guide/en/#avoiding-eval node_modules/@protobufjs/inquire/index.js

function inquire(moduleName) {
try {
        var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval
                      ^
        if (mod && (mod.length || Object.keys(mod).length))
        return mod;
gunta commented 1 year ago

+1

edevil commented 1 year ago

eval() is also not available on certain environments such as Cloudflare Workers, which prevents this module, or anything that depends on it, from being used.

kaiyoma commented 1 year ago

Getting these errors during a Vite build:

00:22:03  common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
00:23:24  common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
00:24:32  common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
00:24:42  common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "common/temp/node_modules/.pnpm/@protobufjs+inquire@1.1.0/node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
oleksandr-dukhovnyy commented 1 year ago

It's not entirely clear why this is needed at all? Why not var mod = require(moduleName);?

Assafz1983 commented 11 months ago

Hi Protobufjs team! Are you planning on removing the usage of eval in the near future?

tsemachh commented 11 months ago

Same goes for Chrome Extensions using Manifest V3 - where it's also forbidden to use eval

whizzzkid commented 10 months ago

☝🏽 absolutely, had to patch to run on MV3.

davideberlein commented 10 months ago

This is a huge issue for us as the page we are integrating into forbids eval via CSP

AntiMoron commented 9 months ago

same here

AntiMoron commented 9 months ago

https://github.com/protobufjs/protobuf.js/pull/1941

I give it a try, more tests are needed.

kaiyoma commented 7 months ago

In addition to the Vite errors, this issue causes runtime CSP errors in Firefox:

Content-Security-Policy: The page’s settings blocked the loading of a resource at eval (“script-src”).

Clicking the link in the browser console error takes you here:

image

ngbrown commented 6 months ago

I use this patch-package file:

patches/protobufjs+7.2.5.patch ```patch diff --git a/node_modules/protobufjs/dist/light/protobuf.js b/node_modules/protobufjs/dist/light/protobuf.js index 5727c45..3004e3d 100644 --- a/node_modules/protobufjs/dist/light/protobuf.js +++ b/node_modules/protobufjs/dist/light/protobuf.js @@ -876,6 +876,10 @@ module.exports = inquire; * @returns {?Object} Required module if available and not empty, otherwise `null` */ function inquire(moduleName) { + // Don't use eval with CSP in a browser: https://github.com/protobufjs/protobuf.js/pull/1548 + if (typeof document !== "undefined") { + return null; + } try { var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval if (mod && (mod.length || Object.keys(mod).length)) diff --git a/node_modules/protobufjs/dist/minimal/protobuf.js b/node_modules/protobufjs/dist/minimal/protobuf.js index 87e6f55..d5e2d9e 100644 --- a/node_modules/protobufjs/dist/minimal/protobuf.js +++ b/node_modules/protobufjs/dist/minimal/protobuf.js @@ -658,6 +658,10 @@ module.exports = inquire; * @returns {?Object} Required module if available and not empty, otherwise `null` */ function inquire(moduleName) { + // Don't use eval with CSP in a browser: https://github.com/protobufjs/protobuf.js/pull/1548 + if (typeof document !== "undefined") { + return null; + } try { var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval if (mod && (mod.length || Object.keys(mod).length)) diff --git a/node_modules/protobufjs/dist/protobuf.js b/node_modules/protobufjs/dist/protobuf.js index cda26c5..012e2f5 100644 --- a/node_modules/protobufjs/dist/protobuf.js +++ b/node_modules/protobufjs/dist/protobuf.js @@ -876,6 +876,10 @@ module.exports = inquire; * @returns {?Object} Required module if available and not empty, otherwise `null` */ function inquire(moduleName) { + // Don't use eval with CSP in a browser: https://github.com/protobufjs/protobuf.js/pull/1548 + if (typeof document !== "undefined") { + return null; + } try { var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval if (mod && (mod.length || Object.keys(mod).length)) diff --git a/node_modules/protobufjs/src/util.js b/node_modules/protobufjs/src/util.js index 6c50899..bd9a61d 100644 --- a/node_modules/protobufjs/src/util.js +++ b/node_modules/protobufjs/src/util.js @@ -199,6 +199,7 @@ util.setProperty = function setProperty(dst, path, value) { return setProp(dst, path, value); }; +if (!util.hasOwnProperty("decorateRoot")){ /** * Decorator root (TypeScript). * @name util.decorateRoot @@ -210,3 +211,4 @@ Object.defineProperty(util, "decorateRoot", { return roots["decorated"] || (roots["decorated"] = new (require("./root"))()); } }); +} ```

The if (!util.hasOwnProperty("decorateRoot")) part is to prevent an error of re-defining the same property when using HMR.

jakub-g commented 5 months ago

To try to move forward with this and close the Content Security Policy threads like #997, could some maintainer explain what problem the usage of eval inside inquire resolves precisely?

❓ Maybe the problem is no longer relevant?

By doing some archeology, it seems that eval was shipped many years ago due to webpack 4 automagically bundling Buffer for the web when it noticed require.

Note that webpack 5 (released Nov 2020) has broke compat on this, and longer automagically ships nodejs polyfills.

So if that's the only reason, I'd suggest to cut a major version of protobufjs (8.0), and remove the usage of eval.

KAMAELUA commented 4 months ago

Any updates? This completely breaks packaging ESM package because require is not defined and this eval is always running it.

alaingiller commented 4 months ago

I also got the problem while using @opentelemetry/exporter-trace-otlp-proto so I switch to @opentelemetry/exporter-trace-otlp-http. Less performant, but more secure at least...

graham-atom commented 2 months ago

I am also seeing this warning when building with vite, any updates?

morzel85 commented 3 weeks ago

Just got this error (July 2024)...

alefduarte commented 1 week ago

2 years have passed and the problem persists...

starseeker00 commented 18 hours ago

I try to use this package in Cloudflare Workers, and have the same trouble with the usage of eval(). Any ideas to solve this problem? Thanks for helping.