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

sign crashes with validator.isValid is not a function for certain objects #945

Open
ej-shafran opened this issue Nov 3, 2023 · 2 comments · May be fixed by #946
Open

sign crashes with validator.isValid is not a function for certain objects #945

ej-shafran opened this issue Nov 3, 2023 · 2 comments · May be fixed by #946

Comments

@ej-shafran
Copy link

Description

Provide a clear and concise description of the issue, including what you expected to happen.

I noticed that every once in a blue moon, my tests for something that calls sign would fail with validator.isValid is not a function. After debugging, I managed to narrow it down. I'm using fast-check for tests, and their fc.object function sometimes generates objects that have keys like "__proto__", "valueOf", or "toString". Whenever an object like this is passed into sign, the validator.isValid error appears.

Reproduction

Most minimal reproduction I could create:

  1. Create a new Node project, installing jsonwebtoken
  2. In a file index.js, add:
const { sign } = require("jsonwebtoken");

console.log(sign({ valueOf: 0 }, "anysecret");
  1. Run node index.js
  2. See error

To reproduce the fast-check test that shows the different ways this error occurs:

  1. Create a new Node project, installing jest, jsonwebtoken, and fast-check.
  2. In a file jwt.test.js, add:
const fc = require("fast-check");
const { sign } = require("jsonwebtoken");

describe("jwt.sign", () => {
  it("should sign any object", () => {
    const prop = fc.property(
      fc.object(),
      fc.base64String({ minLength: 1 }),
      (obj, secret) => {
        const result = sign(obj, secret);
        return !!result;
      },
    );

    fc.assert(prop, { numRuns: 1000, verbose: true });
  });
});
  1. Run npx jest
  2. See that the test does not pass for certain values, and a TypeError: validator.isValid is not a function is thrown

Environment

Please provide the following:

  • Version of this library used: ^9.0.2
  • Version of the platform or framework used, if applicable: N/A
  • Other relevant versions (language, server software, OS, browser): both JS and TS, on Linux, using Node
  • Other modules/plugins/libraries that might be involved: fast-check
@ej-shafran
Copy link
Author

I looked into the source code some more. Seems the issue is that the code iterates over the keys of the object (in validate) and checks for a validator by accessing schema[key]. In the case of keys like toString, __proto__, and valueOf, it does find a value, so the if (!validator) check doesn't trigger. It then tries to call validator.isValid, but since it's a function (or the prototype of the object), it crashes....

@ej-shafran
Copy link
Author

Managed to fix this by doing:

 function validate(schema, allowUnknown, object, parameterName) {
   if (!isPlainObject(object)) {
     throw new Error('Expected "' + parameterName + '" to be a plain object.');
   }
   Object.keys(object)
     .forEach(function(key) {
-      const validator = schema[key];
-      if (!validator) {
+     if (!Object.getOwnPropertyNames(schema).includes(key)) {
         if (!allowUnknown) {
           throw new Error('"' + key + '" is not allowed in "' + parameterName + '"');
         }
         return;
       }
+      const validator = schema[key];
       if (!validator.isValid(object[key])) {
         throw new Error(validator.message);
       }
     });
 }

(Though it's probably better to use Object.hasOwnProperty or lodash.has instead)

@ej-shafran ej-shafran linked a pull request Nov 3, 2023 that will close this issue
4 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

Successfully merging a pull request may close this issue.

1 participant