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

Risk of remote code execution for untrusted schemas #326

Open
koorchik opened this issue Jun 9, 2023 · 4 comments
Open

Risk of remote code execution for untrusted schemas #326

koorchik opened this issue Jun 9, 2023 · 4 comments

Comments

@koorchik
Copy link

koorchik commented Jun 9, 2023

Code

import FastestValidator from 'fastest-validator';
const v = new FastestValidator();

const check = v.compile({
  id: { type: 'number', max: 'console.log("ALERT")' }
});

check({id:123});

will print 'ALERT'.

Ajv validator has similar architecture but is secure for such types of attacks.

It is possible to guard against such type of attack with quoting of parameters which can be done in compile-time
"Ajv Safe code generation" - https://ajv.js.org/codegen.html#safe-code-generation

@icebob
Copy link
Owner

icebob commented Jun 9, 2023

The validation schema is not a user input data, so it's not a real issue and we also don't recommend to do it

@koorchik
Copy link
Author

koorchik commented Jun 10, 2023

Yes, but the risk of remote code execution for untrusted does exist as it was with ajv before (now it is fixed)

Moreover, according to this library docs: "This is as safe as writing code normally and having it compiled by V8 in the usual way.". But it is half true. For example, with Joi validator, you write code but it is safer.

Joi example with code (code is safe):

function prepareValidator(maxId) {
  return Joi.object({
    id: Joi.number().max(maxId),
  });
}

Code is potentially vulnerable with maxId coming from database, for example

function prepareValidator(maxId) {
  return fastestValidator.compile({ 
    id: {type: 'number', max: maxId} 
  });
}

So, compared to code you expect that data is data (string is string, number is number), and code is code. But here is string data can be a code.

@intech
Copy link
Contributor

intech commented Jun 10, 2023

@icebob I think this is a real security problem. Possible attack by merging two vectors:
Prototype pollution (very popular in many packages) + eval injection (this issue).

@koorchik Thx for this report!

@FerX
Copy link
Contributor

FerX commented Jun 20, 2024

without losing performance we could insert a sanitization during compilation

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

4 participants