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

Use of __proto__ seems discouraged #1169

Closed
4 tasks done
gautaz opened this issue Dec 19, 2022 · 8 comments
Closed
4 tasks done

Use of __proto__ seems discouraged #1169

gautaz opened this issue Dec 19, 2022 · 8 comments

Comments

@gautaz
Copy link
Contributor

gautaz commented Dec 19, 2022

Prerequisites

  • I am running the latest version
  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed
  • I'm reporting the issue to the correct repository

Current Behavior

Looking at

// Depending how the schema was create the correct constructor name (from base class) will be locate on __proto__.
target = utils.getConstructorName(schema.__proto__);
if (serviceName === target) {
Object.setPrototypeOf(schema.__proto__, this.ServiceFactory);
return schema;
}
// This is just to handle some idiosyncrasies from Jest.
if (schema._isMockFunction) {
target = utils.getConstructorName(schema.prototype.__proto__);

It seems that the ServiceBroker code relies on __proto__ which seems to be discouraged from a security point of view.

__proto__ is even unavailable on deno, see denoland/deno#4324.
This means that moleculer currently cannot be used with node --disable-proto=delete or deno even with the latest addition of npm specifiers.

Expected Behavior

The expectation is that moleculer runs with the --disable-proto flag provided by node or even deno with npm specifiers.

Failure Information

Currently with node --disable-proto=delete, the Moleculer microservice sample fails with:

[2022-12-19T10:43:07.628Z] FATAL nixos-37367/BROKER: Unable to create ServiceBroker. TypeError: Cannot read properties of undefined (reading 'prototype')
    at Object.getConstructorName (XXX/node-test/node_modules/moleculer/src/utils.js:477:20)
    at ServiceBroker.normalizeSchemaConstructor (XXX/node-test/node_modules/moleculer/src/service-broker.js:1778:18)
    at ServiceBroker.createService (XXX/node-test/node_modules/moleculer/src/service-broker.js:830:17)
    at ServiceBroker.registerInternalServices (XXX/node-test/node_modules/moleculer/src/service-broker.js:978:8)
    at new ServiceBroker (XXX/node-test/node_modules/moleculer/src/service-broker.js:312:10)
    at file:///XXX/node-test/index.js:4:16
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
    at async loadESM (node:internal/process/esm_loader:88:5)

With deno:

[2022-12-19T10:07:06.801Z] FATAL nixos-36342/BROKER: Unable to create ServiceBroker. TypeError: Cannot read properties of undefined (reading 'prototype')
    at Object.getConstructorName (file:///XXX/.cache/deno/npm/registry.npmjs.org/moleculer/0.14.27/src/utils.js:477:20)
    at ServiceBroker.normalizeSchemaConstructor (file:///XXX/.cache/deno/npm/registry.npmjs.org/moleculer/0.14.27/src/service-broker.js:1780:18)
    at ServiceBroker.createService (file:///XXX/.cache/deno/npm/registry.npmjs.org/moleculer/0.14.27/src/service-broker.js:830:17)
    at ServiceBroker.registerInternalServices (file:///XXX/.cache/deno/npm/registry.npmjs.org/moleculer/0.14.27/src/service-broker.js:978:8)
    at new ServiceBroker (file:///XXX/.cache/deno/npm/registry.npmjs.org/moleculer/0.14.27/src/service-broker.js:312:10)
    at file:///XXX/deno-test/index.js:4:16

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Initialize a project with the Moleculer microservice sample
  2. Run the sample with node --disable-proto=delete or deno

Reproduce code snippet

See the Moleculer microservice sample.

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Moleculer version: 0.14.27
  • NodeJS version: v16.16.0
  • Operating System: NixOS 22.11

Failure Logs

See Failure Information.

@gautaz
Copy link
Contributor Author

gautaz commented Dec 19, 2022

Hello @icebob, I've just pushed a PR to fix this issue.
I have no idea if this is the best way to fix it so feel free to comment :-).

@intech
Copy link
Member

intech commented Dec 19, 2022

@gautaz Ufff, this is a "prototype pollution" vulnerability. Thanks for your contribution!

icebob added a commit that referenced this issue Jan 2, 2023
fix for #1169: getPrototypeOf instead of __proto__
@icebob
Copy link
Member

icebob commented Jan 2, 2023

Thanks!

@icebob icebob closed this as completed Jan 2, 2023
@gautaz
Copy link
Contributor Author

gautaz commented Jan 3, 2023

Thanks!

@icebob You're welcome!
In fact I should thank you for being the instigator of the great piece of software that Moleculer is :-).

@icebob
Copy link
Member

icebob commented Jan 7, 2023

@gautaz Could you start Moleculer in Deno?

@gautaz
Copy link
Contributor Author

gautaz commented Jan 9, 2023

Hello @icebob, yes !

❯ deno run --allow-env --allow-read --allow-sys index.js
[2023-01-09T12:55:46.241Z] INFO  lh22-thild-2396/BROKER: Moleculer v0.14.28 is starting...
[2023-01-09T12:55:46.251Z] INFO  lh22-thild-2396/BROKER: Namespace: <not defined>
[2023-01-09T12:55:46.251Z] INFO  lh22-thild-2396/BROKER: Node ID: lh22-thild-2396
[2023-01-09T12:55:46.252Z] INFO  lh22-thild-2396/REGISTRY: Strategy: RoundRobinStrategy
[2023-01-09T12:55:46.253Z] INFO  lh22-thild-2396/REGISTRY: Discoverer: LocalDiscoverer
[2023-01-09T12:55:46.254Z] INFO  lh22-thild-2396/BROKER: Serializer: JSONSerializer
[2023-01-09T12:55:46.264Z] INFO  lh22-thild-2396/BROKER: Validator: FastestValidator
[2023-01-09T12:55:46.266Z] INFO  lh22-thild-2396/BROKER: Registered 13 middleware(s).
[2023-01-09T12:55:46.280Z] INFO  lh22-thild-2396/REGISTRY: '$node' service is registered.
[2023-01-09T12:55:46.281Z] INFO  lh22-thild-2396/REGISTRY: 'math' service is registered.
[2023-01-09T12:55:46.281Z] INFO  lh22-thild-2396/$NODE: Service '$node' started.
[2023-01-09T12:55:46.281Z] INFO  lh22-thild-2396/MATH: Service 'math' started.
[2023-01-09T12:55:46.282Z] INFO  lh22-thild-2396/BROKER: ✔ ServiceBroker with 2 service(s) started successfully in 5ms.
5 + 3 = 8
[2023-01-09T12:55:46.388Z] INFO  lh22-thild-2396/$NODE: Service '$node' stopped.
[2023-01-09T12:55:46.388Z] INFO  lh22-thild-2396/MATH: Service 'math' stopped.
[2023-01-09T12:55:46.389Z] INFO  lh22-thild-2396/BROKER: ServiceBroker is stopped. Good bye.

With a simple test index.js:

import { ServiceBroker } from "npm:moleculer@0.14.28";

// Create a broker
const broker = new ServiceBroker();

// Create a service
broker.createService({
        name: "math",
        actions: {
                add(ctx) {
                        return Number(ctx.params.a) + Number(ctx.params.b);
                }
        }
});

// Start broker
broker.start()
// Call service
        .then(() => broker.call("math.add", { a: 5, b: 3 }))
        .then(res => console.log("5 + 3 =", res))
        .catch(err => console.error(`Error occurred! ${err.message}`));

@icebob
Copy link
Member

icebob commented Jan 12, 2023

Yeah this kind of project works, but I tried to start a project with Runner but it didn't work.

@gautaz
Copy link
Contributor Author

gautaz commented Jan 13, 2023

Ah ok, I do not use the runner as I rely on kubernetes or swarm to manage my instances.
In fact I have always wondered why the runner was part of the core project and was not located in its own npm module to reduce the footprint and dependencies when only the broker is needed (but that is a bit off topic, sorry, that would certainly better fit in #433).

@gautaz gautaz mentioned this issue Jan 15, 2023
5 tasks
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

No branches or pull requests

3 participants